feat(semver-validation): rewrite as a composite over semstat

[sydorovdmytro]

The node24 action answered "is this parseable" and nothing else, so classification and ordering were reimplemented in bash in vcluster-release, promote-release and prerelease-setup, none of them agreeing on precedence. This makes semstat the one implementation: the action downloads it, verifies it against the release checksums, and runs it.

is_valid, parsed_version and error_message behave as before, down to the input trimming core.getInput did and the whitespace-only-vs-missing distinction. New: is_stable, release_type, flat major/minor/patch/prerelease/build, and an optional compare_to giving comparison and is_greater. npm semver, dist/ and the node24 runner are gone.

The checksum is a transfer check, not a signature check: semstat is ours, so the sigstore bundle the release also publishes is left alone.

This needs a new semver-validation/v4 rather than an advance of v3, because it adds runner requirements: Linux or macOS with network egress plus curl, tar, sha256sum and jq. v1 through v3 stay on the node action.

Closes DEVOPS-1335

Test plan

• make test-semver-validation — 63 bats tests over both scripts, including checksum mismatch, an archive with no semstat member, a binary reporting the wrong version, arch mapping, dash-leading versions, input trimming, and newline injection into outputs and workflow commands.
• Ran both scripts locally against the published semstat v0.0.2: downloaded, verified, and reported v4.9.0-rc.2 as rc, below v4.9.0.
• Checked -1.2.3, " v2.1.0 ", " " and "" against real semstat; outputs match the node action on each.
• make lint clean, docs regenerated with make generate-docs.
• test-semver-validation.yaml gains a job that runs the action end to end and asserts the three old outputs verbatim plus the new ones.

[sydorovdmytro]

/vcluster-review

[vcluster-pr-approver]

🔍 The agent swarm is reviewing PR #231. I'll post the results here when it's done.

Admins can watch the run live

[loft-bot]

Panel review: 2 blocking, 5 quality notes, 10 lanes. Both blocking findings were reproduced by running the scripts, so neither is something the nine green checks would have caught.

Blocking concerns

• .github/actions/semver-validation/src/report.sh:74 — caller-supplied version/compare_to reach five plain log echoes unsanitized, so a newline forges workflow commands; ::stop-commands:: disables annotations for the rest of the job.
• .github/actions/semver-validation/src/report.sh:79 — every non-zero semstat parse exit is reported as an invalid version, so a crashing binary answers is_valid=false for a valid tag on a green step.

What was checked

Checked: correctness, security, test-quality, operability, architecture, reuse, dead-code, typos, infra, PR metadata, and cross-PR file conflicts. Skipped: the four e2e rubric lanes, gap-analysis, and entry-point fidelity (no e2e*/ suite, no Go unit tests, no customer-bug link), plus layout coherence (not a restructure-shaped diff). Verified and deliberately not raised: the checksum path fails closed on both download arms; all 12 declared outputs are written on every one of the six exit paths; the semstat_version Renovate annotation matches this repo's custom manager; both scripts are committed 100755; the README's gt exit-code example behaves as documented; is_stable vs release_type == 'stable' never disagree. The reuse lane ran but was limited — this sandbox has no git checkout, so it could not read sibling actions to confirm equivalence.

Quality notes (non-blocking)

• consider src/install-semstat.sh:88 — semstat version 2>&1 folds stderr into the compared value, and the exact-equality pins an output shape DEVOPS-1335's command contract never specifies. A stray stderr line rejects a correct binary with a message that reads as a version mismatch; a future release printing v1.2.3 breaks every caller the moment Renovate bumps the pin. Capturing stdout only and asserting the version appears keeps the guarantee without pinning the format.
• consider src/report.sh:127 — empty comparison/is_greater carries three meanings (not requested, compare_to unusable, compare failed) and only the latter two warn. This path logs nothing, so a compare_to expression resolving to empty is indistinguishable from a deliberate omission, and the documented is_greater == 'true' gate quietly stays false on exactly that mis-wiring.
• consider test/install-semstat.bats:172-187 — the two platform tests assert only exit 0, and publish_release() writes byte-identical archives under all four asset names, so any name that resolves passes. Confirmed by mutation: mapping arm64→amd64, and separately Darwin→linux, leaves all 12 tests green. This becomes blocking if the OS/arch-to-asset mapping is ever edited, since no test would catch a wrong-asset regression; today the version cross-check still fails such a build loudly. Recording the requested URL in the curl mock and asserting it would close this.
• consider cross-PR — open #167 (chore(deps): update semver-validation) modifies package.json and package-lock.json, both deleted here. It is obsoleted by this change and worth closing rather than merging after; this PR also removes the Renovate npm rule that produces it.
• nit src/report.sh:41 — the comment reads "unsanitised" while the function it documents two lines below is sanitize, as are all four call sites.

[vcluster-pr-approver]

@sydorovdmytro ✅ Review finished for #231

View the session

[sydorovdmytro]

Both blocking findings were real; fixed in 4654776 and b80f224.

parse exit conflation. Every call now names the exit codes that are answers for it, and anything else fails the step: parse and type accept 0 and 2, validate accepts 0 and 1, compare accepts only 0 because both versions are already known readable by then. A binary that crashed no longer reports is_valid=false for a good tag. Three tests cover it (parse exiting 127, type exiting 139, compare exiting 2).

Unsanitized log lines. All five plain echoes now fold newlines, same as the warnings did. Test asserts neither a forged ::error:: nor a ::stop-commands:: reaches the start of a line, through both version and compare_to.

On the quality notes:

• semstat version — stdout only now, with stderr kept separately for the failure message, and a leading v tolerated so a future release changing its spelling does not break every caller on the next Renovate bump. Tests cover a binary that writes to stderr and one that prints v1.2.6.
• Empty comparison — the omitted-compare_to path now logs that it was omitted, so a compare_to: expression resolving to empty is distinguishable in the log from a deliberate omission.
• Platform tests — the curl mock records every URL and the four platform tests assert the archive name asked for, not just exit 0. Confirmed against the mutation you named: arm64→amd64 now fails tests 9 and 10 rather than passing.
• chore(deps): update semver-validation #167 — closed, with a note pointing here.
• "unsanitised" — the comment is rewritten and now spells it the way the function does.

make test-semver-validation is 35 tests, all green; lint and check-docs clean.

[Piotr1215]

I think there is one input-trimming compatibility issue left before moving the tag.