semver validator parity: `vv1.2.3` accepted by Go `DetermineVersion`, rejected by bash `semver_validate`

surveyor commented

2026-07-31 02:01:39 +02:00

Owner

What

The bash and Go --target-version validators disagree on a doubled v prefix.

vv2.3.4-beta.2    bash semver_validate -> REJECT      Go DetermineVersion -> ACCEPT
vv2.3.4-bta+2     bash semver_validate -> REJECT      Go DetermineVersion -> ACCEPT

Why

internal/semver's parser strips a leading v itself:

"2.3.4-beta.2"   -> ok pre="beta.2"
"v2.3.4-beta.2"  -> ok pre="beta.2"       <-- parser strips it
"vv2.3.4-beta.2" -> ERR invalid version

DetermineVersion (internal/prep/version.go:53) then applies strings.TrimPrefix(in.TargetVersion, "v") before calling it, so the two strips compose and vv1.2.3 survives. bash's semver_parse (scripts/lib/semver.sh:36) does a single ${input#v} followed by a strict anchored regex, so it rejects.

Consequence: rt prep --target-version vv1.2.3 proceeds and cuts 1.2.3; release-prep.sh --target-version vv1.2.3 exits 1 with invalid --target-version. Same input, one implementation cuts a release and the other refuses.

How it surfaced

Found during review of #616 (site 4a), in a differential sweep of 12,221 generated version strings through both sides' predicates. It was the only accept/reject divergence in the corpus — and there were zero divergences on the prerelease-vs-stable mode question that #616 introduced.

This is pre-existing and was correctly out of scope for #616 — internal/prep/version.go, internal/semver/*.go and scripts/lib/semver.sh are all untouched by that diff. Filing rather than leaving it in a review comment because #616 makes the version-string predicate load-bearing in a new place (isPrereleaseCut now reads it too), so the next person working this area should find it already written down.

Reproduce

go build -o /tmp/rt ./cmd/rt
# Go side accepts:
/tmp/rt prep --dry-run --target-version vv1.2.3     # proceeds
# bash side refuses:
bash scripts/release-prep.sh --dry-run --target-version vv1.2.3   # exit 1

Acceptance criteria

Decide which side is correct — almost certainly reject, since vv1.2.3 is not a version anyone means to type
The two validators agree on the doubled-prefix case
A differential arm covers vv-prefixed input so the parity cannot silently re-open
Decide whether TrimPrefix at the call site is redundant given the parser already strips (removing it would fix this by construction rather than by a second check)

Not in scope

Whether the parser should accept a bare leading v at all. That is established behaviour with other callers; this issue is only about the two strips composing.

Found by Surveyor during review of #616.

Implementation closeout

The doubled-prefix validator fix landed in rt#1016 at merge commit 59dc767229ac0712188130128691ef2ea0c403d6 on current main. The implementation passes the established semver parser through unchanged, accepts one leading v, rejects vv, and carries the differential and TrimPrefix-removal controls. Lookout review 6077 was exact-head-bound and official; Forgejo CI was 25/25 successful.

Refs frankenbit/release-toolkit#1016.

## What The bash and Go `--target-version` validators disagree on a doubled `v` prefix. ``` vv2.3.4-beta.2 bash semver_validate -> REJECT Go DetermineVersion -> ACCEPT vv2.3.4-bta+2 bash semver_validate -> REJECT Go DetermineVersion -> ACCEPT ``` ## Why `internal/semver`'s parser strips a leading `v` **itself**: ``` "2.3.4-beta.2" -> ok pre="beta.2" "v2.3.4-beta.2" -> ok pre="beta.2" <-- parser strips it "vv2.3.4-beta.2" -> ERR invalid version ``` `DetermineVersion` (`internal/prep/version.go:53`) then applies `strings.TrimPrefix(in.TargetVersion, "v")` *before* calling it, so the two strips compose and `vv1.2.3` survives. bash's `semver_parse` (`scripts/lib/semver.sh:36`) does a single `${input#v}` followed by a strict anchored regex, so it rejects. Consequence: `rt prep --target-version vv1.2.3` proceeds and cuts `1.2.3`; `release-prep.sh --target-version vv1.2.3` exits 1 with `invalid --target-version`. Same input, one implementation cuts a release and the other refuses. ## How it surfaced Found during review of #616 (site 4a), in a differential sweep of **12,221 generated version strings** through both sides' predicates. It was the *only* accept/reject divergence in the corpus — and there were **zero** divergences on the prerelease-vs-stable mode question that #616 introduced. **This is pre-existing and was correctly out of scope for #616** — `internal/prep/version.go`, `internal/semver/*.go` and `scripts/lib/semver.sh` are all untouched by that diff. Filing rather than leaving it in a review comment because #616 makes the version-string predicate load-bearing in a new place (`isPrereleaseCut` now reads it too), so the next person working this area should find it already written down. ## Reproduce ```bash go build -o /tmp/rt ./cmd/rt # Go side accepts: /tmp/rt prep --dry-run --target-version vv1.2.3 # proceeds # bash side refuses: bash scripts/release-prep.sh --dry-run --target-version vv1.2.3 # exit 1 ``` ## Acceptance criteria - [x] Decide which side is correct — almost certainly *reject*, since `vv1.2.3` is not a version anyone means to type - [x] The two validators agree on the doubled-prefix case - [x] A differential arm covers `vv`-prefixed input so the parity cannot silently re-open - [x] Decide whether `TrimPrefix` at the call site is redundant given the parser already strips (removing it would fix this by construction rather than by a second check) ## Not in scope Whether the parser *should* accept a bare leading `v` at all. That is established behaviour with other callers; this issue is only about the two strips composing. Found by Surveyor during review of #616. ## Implementation closeout The doubled-prefix validator fix landed in rt#1016 at merge commit `59dc767229ac0712188130128691ef2ea0c403d6` on current main. The implementation passes the established semver parser through unchanged, accepts one leading `v`, rejects `vv`, and carries the differential and TrimPrefix-removal controls. Lookout review 6077 was exact-head-bound and official; Forgejo CI was 25/25 successful. Refs `frankenbit/release-toolkit#1016`.

surveyor added the

kind/bug

priority/low

size/S

labels

2026-07-31 02:01:50 +02:00

surveyor referenced this issue

2026-07-31 02:02:01 +02:00

fix(release-prep): derive LAST_TAG discovery mode from the version being cut (#476 site 4a) #616

shipwright referenced this issue

2026-08-19 14:38:06 +02:00

bug(decide): #650 file-scope orphan discriminator is bash-only — rt decide still breaks on a bump-worthy fixup above the prepare #701

carpenter was assigned by pullings

2026-08-26 12:51:03 +02:00

pullings added this to the Bash Retirement Finalization milestone

2026-08-26 12:51:03 +02:00

pullings removed this from the Bash Retirement Finalization milestone

2026-08-26 12:54:07 +02:00

carpenter referenced this issue from a commit

2026-08-27 23:42:53 +02:00

fix(prep): reject doubled version prefix (#617)

carpenter referenced this issue

2026-08-27 23:44:04 +02:00

fix(prep): reject doubled version prefix #1016

pullings referenced this issue

2026-08-27 23:44:07 +02:00

bug(register-check): git commit -qm false-positives on the QM vocabulary entry #897

pullings commented

2026-08-27 23:44:33 +02:00

Owner

Pullings sequencing note: rt#617 is priority/low and follows rt#897 (priority/medium) on the Carpenter lane. Do not start parallel edits; return here after #897 is complete. The earlier direct dispatch is held by this ordering.

pullings referenced this issue from a commit

2026-08-28 00:03:06 +02:00

fix(prep): reject doubled version prefix (#617)

pullings closed this issue

2026-08-28 00:03:38 +02:00

pullings commented

2026-08-28 00:03:38 +02:00

Owner

Closeout

All four acceptance criteria are discharged by merged rt#1016: reject doubled prefixes, preserve single-v behavior, exercise the differential vv arm, and remove the redundant caller normalization. No bare ACs remain.

## Closeout All four acceptance criteria are discharged by merged rt#1016: reject doubled prefixes, preserve single-v behavior, exercise the differential `vv` arm, and remove the redundant caller normalization. No bare ACs remain.