Skip to content

Commit 9865f04

Browse files
authored
Merge pull request #23 from linksplatform/issue-22-d2a2ebbc5352
fix: release pipeline must always publish version greater than already published
2 parents 5c89b12 + ae05383 commit 9865f04

7 files changed

Lines changed: 549 additions & 64 deletions

File tree

Cargo.lock

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.
Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,13 @@
1+
---
2+
bump: patch
3+
---
4+
5+
### Fixed
6+
- Release pipeline now always publishes a version greater than what exists on crates.io
7+
- `publish-crate.rs` now fails (exit 1) when version already exists instead of silently accepting
8+
- `version-and-commit.rs` now queries crates.io for the max published version and ensures the new version exceeds it
9+
- `check-release-needed.rs` now outputs `max_published_version` for diagnostic purposes
10+
11+
### Added
12+
- Case study documentation for issue #22 analyzing the false-positive "already exists" bug
13+
- Experiment script testing version bumping logic against published versions
Lines changed: 133 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,133 @@
1+
# Issue #22: Release pipeline must always publish a version greater than what exists on crates.io
2+
3+
## Problem Statement
4+
5+
When the auto-release pipeline runs and encounters a version that already exists on crates.io,
6+
it treats this as a success ("this is OK") and exits cleanly. This is a false positive: the
7+
pipeline should never silently accept that a version was not published. If the version already
8+
exists, the pipeline must bump to a higher version and publish that instead.
9+
10+
## Timeline of Events (CI Run 23707269823)
11+
12+
1. **2026-03-29T10:45:00Z** - Auto Release job starts after successful build
13+
2. **2026-03-29T10:46:05Z** - `check-release-needed.rs` outputs `should_release=true`, `skip_bump=false`
14+
- 7 changelog fragments exist, so `has_fragments=true`
15+
3. **2026-03-29T10:46:08Z** - `version-and-commit.rs` determines bump type is `minor` (0.2.0 -> 0.3.0... but wait)
16+
- Actually, current version in Cargo.toml is already `0.2.0`
17+
- The bump from `0.2.0` with `minor` would produce `0.3.0`
18+
- BUT the script finds `Tag v0.3.0` does NOT exist... however `Tag v0.2.0` DOES exist
19+
- The log shows: **"Tag v0.2.0 already exists"** - this means the script computed `0.2.0` as new_version
20+
- This happens because Cargo.toml already has `0.2.0`, and the previous release cycle already
21+
created the tag but the fragments were not cleaned up
22+
4. **2026-03-29T10:46:10Z** - `publish-crate.rs` attempts to publish `platform-trees@0.2.0`
23+
5. **2026-03-29T10:46:11Z** - crates.io responds that version already exists
24+
6. **Outcome**: `publish_result=already_exists` - treated as success, but nothing was actually released
25+
26+
## Root Cause Analysis
27+
28+
### Root Cause 1: `publish-crate.rs` treats "already exists" as success
29+
30+
In `publish-crate.rs` lines 170-172:
31+
```rust
32+
if combined.contains("already uploaded") || combined.contains("already exists") {
33+
println!("Version {} already exists on crates.io - this is OK", version);
34+
set_output("publish_result", "already_exists");
35+
}
36+
```
37+
38+
The script exits with code 0 (success) when a version already exists. This was originally
39+
intended as a "soft failure" to handle race conditions, but it masks the real problem: the
40+
pipeline is trying to publish a version that's already published.
41+
42+
**Fix**: Exit with failure (code 1) when version already exists. The pipeline should never
43+
attempt to publish an already-published version - that indicates a bug in the version
44+
bumping logic.
45+
46+
### Root Cause 2: `version-and-commit.rs` doesn't bump past existing tags
47+
48+
When `version-and-commit.rs` computes the new version and finds the tag already exists (line 281),
49+
it enters a "sync" path that only updates Cargo.toml to match the existing tag but does NOT
50+
try a higher version. The script should keep bumping until it finds an unpublished version.
51+
52+
**Fix**: When the computed new version already has a tag, keep incrementing (patch bump) until
53+
finding a version without an existing tag and not published on crates.io.
54+
55+
### Root Cause 3: No crates.io version check before publishing
56+
57+
The pipeline checks crates.io in `check-release-needed.rs` but only for the current Cargo.toml
58+
version (to decide if release is needed). It does NOT check whether the *new* bumped version
59+
is already published. The authoritative check should query the maximum published version and
60+
ensure the new version is strictly greater.
61+
62+
**Fix**: Add a function to `check-release-needed.rs` that queries the latest published version
63+
from crates.io and outputs it, so `version-and-commit.rs` can ensure the bump target exceeds it.
64+
65+
### Root Cause 4: Stale changelog fragments
66+
67+
The 7 changelog fragments from previous releases were not cleaned up after the v0.2.0 release.
68+
This caused `check-release-needed.rs` to think a new release was needed (`has_fragments=true`)
69+
even though the fragments were already processed.
70+
71+
**Note**: This is actually the trigger for the whole issue - if fragments had been cleaned,
72+
`has_fragments` would be `false` and the pipeline would have checked the current version
73+
against crates.io instead.
74+
75+
## Sequence Diagram
76+
77+
```
78+
check-release-needed.rs
79+
|-- has_fragments=true (stale fragments from v0.2.0)
80+
|-- should_release=true, skip_bump=false
81+
v
82+
version-and-commit.rs
83+
|-- current version: 0.2.0 (already at v0.2.0)
84+
|-- bump minor: 0.2.0 -> 0.3.0? NO - bump reads current as 0.2.0
85+
|-- Wait: the bump computes from Cargo.toml version 0.2.0
86+
|-- With fragments requesting "minor": 0.2.0 minor bump = 0.3.0
87+
|-- But CI log says "Tag v0.2.0 already exists" which means new_version=0.2.0
88+
|-- This means the script somehow computed 0.2.0 as the new version
89+
|-- LIKELY: skip_bump was not properly propagated, or Cargo.toml was pre-bumped
90+
v
91+
publish-crate.rs
92+
|-- Tries to publish 0.2.0
93+
|-- crates.io: "already uploaded"
94+
|-- Script: "this is OK" (EXIT 0) <-- BUG: should be failure
95+
v
96+
create-github-release.rs
97+
|-- Release v0.2.0 already exists, skipping
98+
```
99+
100+
## Solutions Implemented
101+
102+
### 1. `publish-crate.rs`: Fail on "already exists"
103+
- Changed `already_exists` from exit 0 to exit 1
104+
- Added `--allow-existing` flag for backwards compatibility if needed
105+
- Clear error message explaining the version must be bumped
106+
107+
### 2. `version-and-commit.rs`: Bump past existing versions
108+
- Added crates.io version checking (query max published version)
109+
- When computed version already has a tag OR is already on crates.io, keep bumping patch
110+
- Ensures the released version is always strictly greater than the latest published version
111+
112+
### 3. `check-release-needed.rs`: Output max published version
113+
- Added query for the latest version published on crates.io
114+
- Outputs `max_published_version` for downstream scripts to use
115+
- Provides better diagnostic information in logs
116+
117+
### 4. Added verbose/debug logging
118+
- All release scripts now support `--verbose` flag
119+
- Detailed logging of version comparison decisions
120+
- Logs crates.io API responses for debugging
121+
122+
## Related Issues and Prior Art
123+
124+
- Issue #10: CI/CD version parsing failure (pre-release semver) - fixed regex
125+
- Issue #12: Dead code + Node.js 20 deprecation - fixed compilation
126+
- Issue #20: Version regression and release failure (HTTP 422) - fixed tag/Cargo.toml sync
127+
128+
## References
129+
130+
- [Semantic Versioning 2.0.0](https://semver.org/)
131+
- [crates.io API documentation](https://crates.io/docs/api)
132+
- [Cargo publish documentation](https://doc.rust-lang.org/cargo/reference/publishing.html)
133+
- CI Run: https://github.com/linksplatform/trees-rs/actions/runs/23707269823/job/69061263454
Lines changed: 135 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,135 @@
1+
#!/usr/bin/env rust-script
2+
//! Test that version bumping logic correctly skips past already-published versions.
3+
//!
4+
//! This experiment verifies the core logic from issue #22:
5+
//! Given a current version and a set of "published" versions, the bump must produce
6+
//! a version strictly greater than the max published version.
7+
//!
8+
//! ```cargo
9+
//! [dependencies]
10+
//! ```
11+
12+
fn bump(major: u32, minor: u32, patch: u32, bump_type: &str) -> (u32, u32, u32) {
13+
match bump_type {
14+
"major" => (major + 1, 0, 0),
15+
"minor" => (major, minor + 1, 0),
16+
_ => (major, minor, patch + 1),
17+
}
18+
}
19+
20+
fn ensure_version_exceeds(
21+
version: (u32, u32, u32),
22+
max_published: Option<(u32, u32, u32)>,
23+
) -> (u32, u32, u32) {
24+
let (major, minor, patch) = version;
25+
26+
if let Some((pub_major, pub_minor, pub_patch)) = max_published {
27+
if (major, minor, patch) <= (pub_major, pub_minor, pub_patch) {
28+
// Set to max_published + patch 1 to exceed it
29+
println!(
30+
" {}.{}.{} <= {}.{}.{}, adjusting to {}.{}.{}",
31+
major, minor, patch,
32+
pub_major, pub_minor, pub_patch,
33+
pub_major, pub_minor, pub_patch + 1
34+
);
35+
return (pub_major, pub_minor, pub_patch + 1);
36+
}
37+
}
38+
39+
(major, minor, patch)
40+
}
41+
42+
fn test_case(
43+
name: &str,
44+
current: (u32, u32, u32),
45+
bump_type: &str,
46+
max_published: Option<(u32, u32, u32)>,
47+
expected: (u32, u32, u32),
48+
) {
49+
let bumped = bump(current.0, current.1, current.2, bump_type);
50+
let final_ver = ensure_version_exceeds(bumped, max_published);
51+
52+
let passed = final_ver == expected;
53+
let status = if passed { "PASS" } else { "FAIL" };
54+
55+
println!(
56+
"[{}] {}: {}.{}.{} --{}-> {}.{}.{} (max_pub={}) => {}.{}.{} (expected {}.{}.{})",
57+
status,
58+
name,
59+
current.0, current.1, current.2,
60+
bump_type,
61+
bumped.0, bumped.1, bumped.2,
62+
max_published.map_or("none".to_string(), |(a,b,c)| format!("{}.{}.{}", a, b, c)),
63+
final_ver.0, final_ver.1, final_ver.2,
64+
expected.0, expected.1, expected.2,
65+
);
66+
67+
if !passed {
68+
std::process::exit(1);
69+
}
70+
}
71+
72+
fn main() {
73+
println!("=== Testing version bumping past published versions ===\n");
74+
75+
// Case 1: Normal bump, version exceeds published -> no adjustment needed
76+
test_case(
77+
"normal minor bump exceeds published",
78+
(0, 2, 0), "minor", Some((0, 2, 0)),
79+
(0, 3, 0),
80+
);
81+
82+
// Case 2: Patch bump produces 0.2.1 which exceeds 0.2.0 -> OK
83+
test_case(
84+
"patch bump past published",
85+
(0, 2, 0), "patch", Some((0, 2, 0)),
86+
(0, 2, 1),
87+
);
88+
89+
// Case 3: Minor bump from 0.1.0 gives 0.2.0 which equals published 0.2.0 -> must exceed
90+
test_case(
91+
"minor bump equal to published",
92+
(0, 1, 0), "minor", Some((0, 2, 0)),
93+
(0, 2, 1),
94+
);
95+
96+
// Case 4: Major bump, no conflict
97+
test_case(
98+
"major bump no conflict",
99+
(0, 5, 3), "major", Some((0, 5, 3)),
100+
(1, 0, 0),
101+
);
102+
103+
// Case 5: No published versions at all
104+
test_case(
105+
"first release",
106+
(0, 1, 0), "patch", None,
107+
(0, 1, 1),
108+
);
109+
110+
// Case 6: Published version much higher - should jump to published+1
111+
test_case(
112+
"published ahead - jumps to max_published+1",
113+
(0, 1, 0), "patch", Some((0, 5, 0)),
114+
(0, 5, 1),
115+
);
116+
117+
// Case 7: THE ACTUAL BUG SCENARIO
118+
// Cargo.toml has 0.2.0, published is 0.2.0, minor bump gives 0.3.0
119+
// 0.3.0 > 0.2.0 so no adjustment needed
120+
test_case(
121+
"issue-22 scenario: Cargo.toml at published version",
122+
(0, 2, 0), "minor", Some((0, 2, 0)),
123+
(0, 3, 0),
124+
);
125+
126+
// Case 8: Edge case - what if version-and-commit somehow produces the same version
127+
// (this was the actual bug: current=0.2.0, bump produced 0.2.0 because of stale state)
128+
test_case(
129+
"issue-22 actual bug: bump produces same as published",
130+
(0, 1, 5), "patch", Some((0, 2, 0)),
131+
(0, 2, 1),
132+
);
133+
134+
println!("\n=== All tests passed! ===");
135+
}

0 commit comments

Comments
 (0)