Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

13 changes: 13 additions & 0 deletions changelog.d/20260329_fix_release_must_exceed_published.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
---
bump: patch
---

### Fixed
- Release pipeline now always publishes a version greater than what exists on crates.io
- `publish-crate.rs` now fails (exit 1) when version already exists instead of silently accepting
- `version-and-commit.rs` now queries crates.io for the max published version and ensures the new version exceeds it
- `check-release-needed.rs` now outputs `max_published_version` for diagnostic purposes

### Added
- Case study documentation for issue #22 analyzing the false-positive "already exists" bug
- Experiment script testing version bumping logic against published versions
133 changes: 133 additions & 0 deletions docs/case-studies/issue-22/analysis.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,133 @@
# Issue #22: Release pipeline must always publish a version greater than what exists on crates.io

## Problem Statement

When the auto-release pipeline runs and encounters a version that already exists on crates.io,
it treats this as a success ("this is OK") and exits cleanly. This is a false positive: the
pipeline should never silently accept that a version was not published. If the version already
exists, the pipeline must bump to a higher version and publish that instead.

## Timeline of Events (CI Run 23707269823)

1. **2026-03-29T10:45:00Z** - Auto Release job starts after successful build
2. **2026-03-29T10:46:05Z** - `check-release-needed.rs` outputs `should_release=true`, `skip_bump=false`
- 7 changelog fragments exist, so `has_fragments=true`
3. **2026-03-29T10:46:08Z** - `version-and-commit.rs` determines bump type is `minor` (0.2.0 -> 0.3.0... but wait)
- Actually, current version in Cargo.toml is already `0.2.0`
- The bump from `0.2.0` with `minor` would produce `0.3.0`
- BUT the script finds `Tag v0.3.0` does NOT exist... however `Tag v0.2.0` DOES exist
- The log shows: **"Tag v0.2.0 already exists"** - this means the script computed `0.2.0` as new_version
- This happens because Cargo.toml already has `0.2.0`, and the previous release cycle already
created the tag but the fragments were not cleaned up
4. **2026-03-29T10:46:10Z** - `publish-crate.rs` attempts to publish `platform-trees@0.2.0`
5. **2026-03-29T10:46:11Z** - crates.io responds that version already exists
6. **Outcome**: `publish_result=already_exists` - treated as success, but nothing was actually released

## Root Cause Analysis

### Root Cause 1: `publish-crate.rs` treats "already exists" as success

In `publish-crate.rs` lines 170-172:
```rust
if combined.contains("already uploaded") || combined.contains("already exists") {
println!("Version {} already exists on crates.io - this is OK", version);
set_output("publish_result", "already_exists");
}
```

The script exits with code 0 (success) when a version already exists. This was originally
intended as a "soft failure" to handle race conditions, but it masks the real problem: the
pipeline is trying to publish a version that's already published.

**Fix**: Exit with failure (code 1) when version already exists. The pipeline should never
attempt to publish an already-published version - that indicates a bug in the version
bumping logic.

### Root Cause 2: `version-and-commit.rs` doesn't bump past existing tags

When `version-and-commit.rs` computes the new version and finds the tag already exists (line 281),
it enters a "sync" path that only updates Cargo.toml to match the existing tag but does NOT
try a higher version. The script should keep bumping until it finds an unpublished version.

**Fix**: When the computed new version already has a tag, keep incrementing (patch bump) until
finding a version without an existing tag and not published on crates.io.

### Root Cause 3: No crates.io version check before publishing

The pipeline checks crates.io in `check-release-needed.rs` but only for the current Cargo.toml
version (to decide if release is needed). It does NOT check whether the *new* bumped version
is already published. The authoritative check should query the maximum published version and
ensure the new version is strictly greater.

**Fix**: Add a function to `check-release-needed.rs` that queries the latest published version
from crates.io and outputs it, so `version-and-commit.rs` can ensure the bump target exceeds it.

### Root Cause 4: Stale changelog fragments

The 7 changelog fragments from previous releases were not cleaned up after the v0.2.0 release.
This caused `check-release-needed.rs` to think a new release was needed (`has_fragments=true`)
even though the fragments were already processed.

**Note**: This is actually the trigger for the whole issue - if fragments had been cleaned,
`has_fragments` would be `false` and the pipeline would have checked the current version
against crates.io instead.

## Sequence Diagram

```
check-release-needed.rs
|-- has_fragments=true (stale fragments from v0.2.0)
|-- should_release=true, skip_bump=false
v
version-and-commit.rs
|-- current version: 0.2.0 (already at v0.2.0)
|-- bump minor: 0.2.0 -> 0.3.0? NO - bump reads current as 0.2.0
|-- Wait: the bump computes from Cargo.toml version 0.2.0
|-- With fragments requesting "minor": 0.2.0 minor bump = 0.3.0
|-- But CI log says "Tag v0.2.0 already exists" which means new_version=0.2.0
|-- This means the script somehow computed 0.2.0 as the new version
|-- LIKELY: skip_bump was not properly propagated, or Cargo.toml was pre-bumped
v
publish-crate.rs
|-- Tries to publish 0.2.0
|-- crates.io: "already uploaded"
|-- Script: "this is OK" (EXIT 0) <-- BUG: should be failure
v
create-github-release.rs
|-- Release v0.2.0 already exists, skipping
```

## Solutions Implemented

### 1. `publish-crate.rs`: Fail on "already exists"
- Changed `already_exists` from exit 0 to exit 1
- Added `--allow-existing` flag for backwards compatibility if needed
- Clear error message explaining the version must be bumped

### 2. `version-and-commit.rs`: Bump past existing versions
- Added crates.io version checking (query max published version)
- When computed version already has a tag OR is already on crates.io, keep bumping patch
- Ensures the released version is always strictly greater than the latest published version

### 3. `check-release-needed.rs`: Output max published version
- Added query for the latest version published on crates.io
- Outputs `max_published_version` for downstream scripts to use
- Provides better diagnostic information in logs

### 4. Added verbose/debug logging
- All release scripts now support `--verbose` flag
- Detailed logging of version comparison decisions
- Logs crates.io API responses for debugging

## Related Issues and Prior Art

- Issue #10: CI/CD version parsing failure (pre-release semver) - fixed regex
- Issue #12: Dead code + Node.js 20 deprecation - fixed compilation
- Issue #20: Version regression and release failure (HTTP 422) - fixed tag/Cargo.toml sync

## References

- [Semantic Versioning 2.0.0](https://semver.org/)
- [crates.io API documentation](https://crates.io/docs/api)
- [Cargo publish documentation](https://doc.rust-lang.org/cargo/reference/publishing.html)
- CI Run: https://github.com/linksplatform/trees-rs/actions/runs/23707269823/job/69061263454
135 changes: 135 additions & 0 deletions experiments/test_version_bumping_past_published.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,135 @@
#!/usr/bin/env rust-script
//! Test that version bumping logic correctly skips past already-published versions.
//!
//! This experiment verifies the core logic from issue #22:
//! Given a current version and a set of "published" versions, the bump must produce
//! a version strictly greater than the max published version.
//!
//! ```cargo
//! [dependencies]
//! ```

fn bump(major: u32, minor: u32, patch: u32, bump_type: &str) -> (u32, u32, u32) {
match bump_type {
"major" => (major + 1, 0, 0),
"minor" => (major, minor + 1, 0),
_ => (major, minor, patch + 1),
}
}

fn ensure_version_exceeds(
version: (u32, u32, u32),
max_published: Option<(u32, u32, u32)>,
) -> (u32, u32, u32) {
let (major, minor, patch) = version;

if let Some((pub_major, pub_minor, pub_patch)) = max_published {
if (major, minor, patch) <= (pub_major, pub_minor, pub_patch) {
// Set to max_published + patch 1 to exceed it
println!(
" {}.{}.{} <= {}.{}.{}, adjusting to {}.{}.{}",
major, minor, patch,
pub_major, pub_minor, pub_patch,
pub_major, pub_minor, pub_patch + 1
);
return (pub_major, pub_minor, pub_patch + 1);
}
}

(major, minor, patch)
}

fn test_case(
name: &str,
current: (u32, u32, u32),
bump_type: &str,
max_published: Option<(u32, u32, u32)>,
expected: (u32, u32, u32),
) {
let bumped = bump(current.0, current.1, current.2, bump_type);
let final_ver = ensure_version_exceeds(bumped, max_published);

let passed = final_ver == expected;
let status = if passed { "PASS" } else { "FAIL" };

println!(
"[{}] {}: {}.{}.{} --{}-> {}.{}.{} (max_pub={}) => {}.{}.{} (expected {}.{}.{})",
status,
name,
current.0, current.1, current.2,
bump_type,
bumped.0, bumped.1, bumped.2,
max_published.map_or("none".to_string(), |(a,b,c)| format!("{}.{}.{}", a, b, c)),
final_ver.0, final_ver.1, final_ver.2,
expected.0, expected.1, expected.2,
);

if !passed {
std::process::exit(1);
}
}

fn main() {
println!("=== Testing version bumping past published versions ===\n");

// Case 1: Normal bump, version exceeds published -> no adjustment needed
test_case(
"normal minor bump exceeds published",
(0, 2, 0), "minor", Some((0, 2, 0)),
(0, 3, 0),
);

// Case 2: Patch bump produces 0.2.1 which exceeds 0.2.0 -> OK
test_case(
"patch bump past published",
(0, 2, 0), "patch", Some((0, 2, 0)),
(0, 2, 1),
);

// Case 3: Minor bump from 0.1.0 gives 0.2.0 which equals published 0.2.0 -> must exceed
test_case(
"minor bump equal to published",
(0, 1, 0), "minor", Some((0, 2, 0)),
(0, 2, 1),
);

// Case 4: Major bump, no conflict
test_case(
"major bump no conflict",
(0, 5, 3), "major", Some((0, 5, 3)),
(1, 0, 0),
);

// Case 5: No published versions at all
test_case(
"first release",
(0, 1, 0), "patch", None,
(0, 1, 1),
);

// Case 6: Published version much higher - should jump to published+1
test_case(
"published ahead - jumps to max_published+1",
(0, 1, 0), "patch", Some((0, 5, 0)),
(0, 5, 1),
);

// Case 7: THE ACTUAL BUG SCENARIO
// Cargo.toml has 0.2.0, published is 0.2.0, minor bump gives 0.3.0
// 0.3.0 > 0.2.0 so no adjustment needed
test_case(
"issue-22 scenario: Cargo.toml at published version",
(0, 2, 0), "minor", Some((0, 2, 0)),
(0, 3, 0),
);

// Case 8: Edge case - what if version-and-commit somehow produces the same version
// (this was the actual bug: current=0.2.0, bump produced 0.2.0 because of stale state)
test_case(
"issue-22 actual bug: bump produces same as published",
(0, 1, 5), "patch", Some((0, 2, 0)),
(0, 2, 1),
);

println!("\n=== All tests passed! ===");
}
Loading