fix(release): validate missing registry version arguments

[mikevillari]

Summary

A missing version argument previously triggered Bash's unbound-variable error and exit 1. Default the argument before stripping the optional v prefix, and use a safe expansion in the diagnostic too, so missing/empty input receives the existing semantic-version error and exit 2.

Related Issue

Closes #396.

Validation

• Tests added or updated — missing, empty, and malformed input in the existing release rehearsal test.
• Security impact considered.
• Trust boundary preserved.
• CI passes — awaiting upstream CI; full local CI did not pass.
• Wrote the regression first: the malformed input passed, then the no-argument case failed with expected exit 2 / actual exit 1. After the change, bash tests/release/release-rehearsal.test.sh passes.
• python3 scripts/check_evidence_claims.py, Bash syntax checks, and git diff --check pass.
• Ran bash scripts/ci-local.sh --no-postgres: 19 checks passed, eight failed. Failures: Rust/frontend dependency DNS access; existing GNU sed/grep expectations on macOS (four tests); setup test's Unix socket bind denied by sandbox. Optional lint/audit/nextest tools were absent. No Rust files changed.
• Tests ran with an isolated Bash 5.2.37 because macOS Bash 3.2 cannot run existing mapfile/associative-array scripts.

Notes for Reviewers

Audited other top-level positional-argument handling: check_release_versions.sh, check_public_claims.sh, markdown-link-files.sh, test_baseline.sh, and check_no_secrets.sh already use default expansions. release_rehearsal.sh and ci-local.sh guard their argument loops; release_rehearsal also checks for a missing --output value.

No release, registry publication, runtime API, or schema changes. This repairs the existing invalid-input contract, so no documentation changes were needed.

AI-assisted with Codex; the diff and test results were inspected by the agent. No independent human-review claim. Two browser commits can be squashed.

[vladimirrott]

Approved at 0ba6e074a69678b32b0ffb4ec0c07b36c94cb45d.

You wrote the regression before the fix and it discriminates. That is the whole ask on this tracker, and it is the part most small fixes skip.

maintainer screen 398 returns DO NOT EXECUTE, so nothing ran on the host. All of the below ran in docker.io/library/bash:5 under rootless podman, --network=none, tree from git archive 0ba6e074.

Reverting your two lines turns the test red

$ out=$(bash /home/entropia/.local/state/sysknife-maint/pt.sh p398 bash -c "$(cat /tmp/sk398proof.sh)" 2>&1); rc=$?; printf '%s\n' "$out" | grep -v '^time='; echo "=== wrapper exit=$rc ==="
### 1. the PR's test, unmutated
ERROR: required tool is missing: cargo
exit=1

### 2. revert the production hunk (back to main's two lines) and re-run
#!/usr/bin/env bash
set -euo pipefail

version="${1#v}"
[[ "$version" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]] || {
    printf 'ERROR: expected a semantic version, got %q\n' "$1" >&2
    exit 2
}
--- confirm the revert matches main byte for byte on those lines ---
4:version="${1#v}"
FAIL: registry preflight expected exit 2, got 1: /repo/scripts/check_registry_versions.sh: line 4: 1: unbound variable
exit=1

### 3. which of the three assertions discriminates? run each against the reverted script
arg=[nope] exit=2  out=ERROR: expected a semantic version, got nope
arg=[NOARG] exit=1  out=scripts/check_registry_versions.sh: line 4: 1: unbound variable
arg=[]     exit=2  out=ERROR: expected a semantic version, got ''

Run 1's required tool is missing: cargo comes from scripts/release_rehearsal.sh:51, which the test reaches after your three assertions. Your assertions sit at the top of the file, so they had already passed when that fired; the container has no cargo, which is my environment and not your code.

Run 3 is the part worth naming. Of the three cases you added, only the bare assert_invalid_registry_version discriminates: nope and '' both take the regex branch and exit 2 in either version. Keep all three as anchors, but the coverage is one case wide rather than three. Your status=$? in the else branch reads the command substitution's own status rather than a pipeline's tail, which is the mistake this repo keeps making, and you avoided it.

Scope, so the fix is not read as bigger than it is

The only automated caller cannot reach the condition you fixed:

$ grep -rn "check_registry_versions" --include=*.yml --include=*.sh --include=*.md . | grep -v '^./target'
.github/workflows/release.yml:47:        run: bash scripts/check_registry_versions.sh "${{ github.ref_name }}"
tests/release/release-rehearsal.test.sh:39:grep -Fq 'check_registry_versions.sh' "$release_workflow"

release.yml triggers on push: tags: 'v[0-9]+.[0-9]+.[0-9]+', so github.ref_name is always a non-empty tag. This improves the message a maintainer sees running the script by hand and closes no release-time hazard. Worth saying out loud so nobody later reads the CHANGELOG and thinks a tag could have shipped on an empty version.

Your reviewer note listing the other five scripts you audited is the right instinct, and it saved me repeating it.

The merge is blocked by my tooling

Both changed files are .sh, so the shell suite covers them, and it still refused:

$ maintainer-merge verify 398 0ba6e074a69678b32b0ffb4ec0c07b36c94cb45d "tests/release/release-rehearsal.test.sh" 's/^version="\${1:-}"$/version="${1#v}"/'
  suite: shell (docker.io/library/python:3.12-slim under podman)
  running 'tests/release/release-rehearsal.test.sh' unmutated
maintainer-merge: the test does not pass unmutated (rc=1); nothing to prove yet
    ERROR: required tool is missing: cargo

The suite image carries bash and python3. scripts/release_rehearsal.sh:51 requires cargo node npm sha256sum file, so this test can never pass clean there and no receipt is earnable. Nothing for you to do: the fix is to the image rather than to your branch, and it is on my list for tonight.

No CHANGELOG entry needed: this restores an existing invalid-input contract rather than changing one.

Next

#238 is a good fit and I say more about it on your other PR. Both of your open PRs are about a guard that reports a guarantee it is not providing, which is the most useful instinct anyone can bring here.

[vladimirrott]

Merged as 7fe0c23, closing #396.

You left the CI checkbox unticked and named the local run you could not finish, instead of ticking it and hoping. I read those checklists closely. That cost you nothing and told me where to look.

Workspace at 5673d20:

$ cargo nextest run --workspace --locked
     Summary [   8.585s] 1851 tests run: 1851 passed, 6 skipped

#395 is yours with #399 open against it. I released #238 and #239 on your word, and saying so before you started rather than after is the version of that which costs nobody an evening.

Separately, with no pressure attached. Two of your PRs now sit on the scripts a contributor runs by hand. sysknife doctor is the read-only end of that surface: it reads the host and changes nothing on it. If you ever run it on a machine of your own, I would want to hear what it got wrong. Not a condition of anything.