Fix the issues from the Codex audit

.github/workflows/release.yml

@@ -5,18 +5,53 @@ on:
     tags:
       - '*'
 
-# Three independent channels. The GitHub release asset and the Docker image
-# must never be hostage to Maven Central: for 4.9 the Central publish queue
-# outlasted the plugin's poll window, the job "failed", and the release and
-# docker steps were skipped while Central went on to publish anyway.
+# Three independent publish channels, all gated on one `test` job.
+#
+# The independence is between the channels, not from the tests. The GitHub release
+# asset and the Docker image must never be hostage to Maven Central: for 4.9 the
+# Central publish queue outlasted the plugin's poll window, the job "failed", and
+# the release and docker steps were skipped while Central went on to publish anyway.
+# So they still do not depend on each other -- but every one of them now depends on
+# `test`, because until it did, `native` and `central` had no `needs:` at all and
+# both build with -DskipTests. A tagged commit with failing tests published native
+# distributions and a Maven Central artifact; only `docker` was gated, and only
+# incidentally, by depending on `build`.
+#
+# `test` runs `verify` rather than `test`, so the shaded-jar smoke test and JaCoCo
+# run here too -- a release is the one place where the packaged artifact being
+# runnable has to be checked before anything is uploaded.
+#
 # Workflow permissions are least-privilege by default (see the maven-publish
-# hardening); only the job that creates the GitHub release gets write access.
+# hardening); only the jobs that create the GitHub release get write access.
 permissions:
   contents: read
 
 jobs:
+  # The gate. Nothing is published unless this passes.
+  test:
+    runs-on: ubuntu-latest
+
+    steps:
+    - name: Checkout code
+      uses: actions/checkout@v4
+
+    - name: Set up JDK 17
+      uses: actions/setup-java@v4
+      with:
+        java-version: '17'
+        distribution: 'temurin'
+        cache: 'maven'
+
+    # No versions:set: the tests do not depend on the version, and the smoke test
+    # resolves the jar through ${project.build.finalName} rather than globbing.
+    # -Dgpg.skip: signing belongs to the `central` job, which has the key. maven-gpg-plugin
+    # binds to verify, so without this the gate fails on a missing key rather than on a test.
+    - name: Verify
+      run: mvn -B verify -Dgpg.skip=true
+
   build:
     runs-on: ubuntu-latest
+    needs: test
     permissions:
       contents: write   # creates the release and uploads the jar asset
 
@@ -69,6 +104,7 @@ jobs:
   # matrix. Each build is verified before upload, because a missing module fails at
   # runtime rather than at build time.
   native:
+    needs: test
     permissions:
       contents: write
     strategy:
@@ -79,7 +115,10 @@ jobs:
             asset: testingbot-tunnel-linux-x64
           - os: ubuntu-24.04-arm
             asset: testingbot-tunnel-linux-arm64
-          - os: macos-13
+          # macos-13 was retired by GitHub on 2025-12-04; macos-15-intel is the
+          # Intel runner that replaced it. jlink emits a runtime for the machine it
+          # runs on, so this entry is what makes the x64 macOS build x64.
+          - os: macos-15-intel
             asset: testingbot-tunnel-macos-x64
           - os: macos-latest
             asset: testingbot-tunnel-macos-arm64
@@ -190,6 +229,7 @@ jobs:
 
   central:
     runs-on: ubuntu-latest
+    needs: test
 
     steps:
     - name: Checkout code
@@ -212,7 +252,7 @@ jobs:
         TAG_VERSION=${GITHUB_REF#refs/tags/v}
         mvn versions:set -DnewVersion=${TAG_VERSION} -DgenerateBackupPoms=false
 
-    # Tests run in the build job; this job only deploys.
+    # Tests run in the `test` job this depends on; this job only deploys.
     - name: Deploy to Maven Central
       run: mvn -B clean deploy -DskipTests -Dgpg.passphrase=${{ secrets.GPG_PASSPHRASE }}
       env:
@@ -222,7 +262,10 @@ jobs:
 
   docker:
     runs-on: ubuntu-latest
-    needs: build
+    # `test`, not `build`: the point was never to wait for the release to be created,
+    # only not to publish an image from a failing build. Naming the gate directly says
+    # that, and lets the image build alongside the release rather than after it.
+    needs: test
 
     steps:
     - name: Checkout code

.github/workflows/test.yml

@@ -74,8 +74,14 @@ jobs:
         key: ${{ runner.os }}-m2-${{ hashFiles('**/pom.xml') }}
         restore-keys: ${{ runner.os }}-m2
 
+    # verify, not package: it adds the shaded-jar smoke test and the JaCoCo report,
+    # which is what the release gate runs. Building with `package` here meant the
+    # first time a release ever checked the packaged jar was actually runnable was
+    # after the tag had been pushed.
+    # -Dgpg.skip: maven-gpg-plugin binds `sign` to the verify phase for the Maven Central
+    # deploy, and there is no signing key here. Only the `central` release job needs it.
     - name: Build project
-      run: mvn clean package
+      run: mvn clean verify -Dgpg.skip=true
 
     - name: Upload build artifacts
       uses: actions/upload-artifact@v7

dist/build-runtime.sh

@@ -14,8 +14,27 @@ HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 ROOT="$(cd "$HERE/.." && pwd)"
 JAVA_HOME="${JAVA_HOME:-$(/usr/libexec/java_home -v 17 2>/dev/null || dirname "$(dirname "$(readlink -f "$(command -v java)")")")}"
 
-JAR="$(ls "$ROOT"/target/TestingBotTunnel-*-shaded.jar 2>/dev/null | head -1)"
-[ -z "$JAR" ] && { echo "No shaded jar. Run: mvn package"; exit 1; }
+# The newest shaded jar, and only when it is unambiguous. `ls | head -1` took the
+# alphabetically first, so after a version bump without `mvn clean` it silently picked
+# the *older* artifact -- 5.10 sorts before 5.9, and the run then verified the previous
+# release while reporting the new one.
+select_shaded_jar() {
+  local root="$1"
+  local jars=()
+  while IFS= read -r line; do jars+=("$line"); done < <(
+    ls -t "$root"/target/TestingBotTunnel-*-shaded.jar 2>/dev/null
+  )
+  if [ "${#jars[@]}" -eq 0 ]; then
+    return 1
+  fi
+  if [ "${#jars[@]}" -gt 1 ]; then
+    echo "Warning: several shaded jars in target/; using the newest ($(basename "${jars[0]}"))." >&2
+    echo "         Run 'mvn clean package' if that is not what you meant." >&2
+  fi
+  printf '%s\n' "${jars[0]}"
+}
+
+JAR="$(select_shaded_jar "$ROOT")" || { echo "No shaded jar. Run: mvn package"; exit 1; }
 
 case "$(uname -s)" in
   Darwin) OS=macos;;

dist/verify-runtime.sh

@@ -13,11 +13,28 @@ set -uo pipefail
 HERE="$(cd "$(dirname "${BASH_SOURCE[0]}")" && pwd)"
 DIST="${1:-}"
 if [ -z "$DIST" ]; then
-  DIST="$(ls -d "$HERE"/testingbot-tunnel-*-*/ 2>/dev/null | head -1)"
+  # -t: newest first. Alphabetical order picked an older build when several are present.
+  DIST="$(ls -dt "$HERE"/testingbot-tunnel-*-*/ 2>/dev/null | head -1)"
 fi
 [ -z "$DIST" ] && { echo "usage: verify-runtime.sh <dist-dir>"; exit 1; }
-LAUNCHER="$DIST/bin/testingbot-tunnel"
-[ -x "$LAUNCHER" ] || { echo "launcher not found: $LAUNCHER"; exit 1; }
+# build-runtime.sh emits a .cmd launcher on Windows and a shell script everywhere
+# else. This looked only for the shell script and required it to be executable, so
+# the Windows leg of the release matrix could never pass -- it failed at this line,
+# before verifying anything, on every tagged build.
+case "$(uname -s)" in
+  MINGW*|MSYS*|CYGWIN*) IS_WINDOWS=1 ;;
+  *)                    IS_WINDOWS=0 ;;
+esac
+
+if [ "$IS_WINDOWS" = "1" ]; then
+  LAUNCHER="$DIST/bin/testingbot-tunnel.cmd"
+  # Not -x: a .cmd carries no Unix executable bit, and whether the MSYS layer
+  # synthesises one is not something to depend on.
+  [ -f "$LAUNCHER" ] || { echo "launcher not found: $LAUNCHER"; exit 1; }
+else
+  LAUNCHER="$DIST/bin/testingbot-tunnel"
+  [ -x "$LAUNCHER" ] || { echo "launcher not found: $LAUNCHER"; exit 1; }
+fi
 
 WORK="$(mktemp -d "${TMPDIR:-/tmp}/tb-runtime.XXXXXX")"
 PID=""
@@ -41,7 +58,31 @@ ok()  { PASS=$((PASS+1)); printf '  \033[32m✓\033[0m %s\n' "$1"; }
 bad() { FAIL=$((FAIL+1)); printf '  \033[31m✗\033[0m %s — %s\n' "$1" "$2"; }
 
 # Deliberately strip the environment so a system JDK cannot rescue a missing module.
-run_isolated() { env -i HOME="$HOME" PATH=/usr/bin:/bin "$@"; }
+#
+# Kept as an array rather than only a function: the live-tunnel launch below has to
+# invoke it directly, because backgrounding a shell function makes $! the wrapping
+# subshell and killing that orphans the JVM underneath it, which then holds a slot
+# against the account's concurrent-tunnel limit.
+#
+# TESTINGBOT_KEY and TESTINGBOT_SECRET are carried through when set. They were not,
+# which made the credentialed section below unreachable by the means it tests for:
+# the guard admitted a run because the variables were set, and then launched the
+# tunnel with an environment that no longer contained them. Only a ~/.testingbot
+# file ever actually exercised the live checks.
+if [ "$IS_WINDOWS" = "1" ]; then
+  # env -i is not usable here: the launcher is a .cmd and needs the Windows command
+  # processor, which is located through the very environment that would be cleared.
+  # Strip what could actually rescue a missing module instead -- a system JDK on
+  # PATH or at JAVA_HOME -- which is what the isolation is for.
+  ISOLATED=(env "JAVA_HOME=" \
+      "PATH=$(printf '%s' "$PATH" | tr ':' '\n' | grep -viE 'jdk|jre|/java' | paste -sd: -)")
+else
+  ISOLATED=(env -i "HOME=$HOME" PATH=/usr/bin:/bin)
+fi
+[ -n "${TESTINGBOT_KEY:-}" ] && ISOLATED+=("TESTINGBOT_KEY=$TESTINGBOT_KEY")
+[ -n "${TESTINGBOT_SECRET:-}" ] && ISOLATED+=("TESTINGBOT_SECRET=$TESTINGBOT_SECRET")
+
+run_isolated() { "${ISOLATED[@]}" "$@"; }
 
 echo "Verifying $(basename "$DIST")"
 
@@ -59,11 +100,11 @@ if [ -z "${TESTINGBOT_KEY:-}" ] && [ ! -f "$HOME/.testingbot" ]; then
   echo "  - live tunnel checks skipped (no credentials)"
 else
   MPORT="$(python3 -c 'import socket;s=socket.socket();s.bind(("127.0.0.1",0));print(s.getsockname()[1]);s.close()')"
-  # Launch directly rather than through run_isolated: backgrounding a shell function
-  # makes $! the wrapping subshell, and killing that orphans the JVM underneath it --
-  # which then keeps holding a tunnel slot on the account.
-  env -i HOME="$HOME" PATH=/usr/bin:/bin \
-      "$LAUNCHER" --readyfile "$WORK/ready" --metrics-port "$MPORT" > "$WORK/tunnel.log" 2>&1 &
+  # The array directly, not run_isolated: backgrounding a shell function makes $! the
+  # wrapping subshell, and killing that orphans the JVM underneath it -- which then
+  # keeps holding a tunnel slot on the account.
+  "${ISOLATED[@]}" "$LAUNCHER" --readyfile "$WORK/ready" --metrics-port "$MPORT" \
+      > "$WORK/tunnel.log" 2>&1 &
   PID=$!
   for _ in $(seq 1 120); do [ -f "$WORK/ready" ] && break; kill -0 $PID 2>/dev/null || break; sleep 1; done
 

e2e/run-e2e.sh

@@ -33,8 +33,27 @@ ROOT="$(cd "$HERE/.." && pwd)"
 # shellcheck source=e2e/webdriver.sh
 source "$HERE/webdriver.sh"
 
-JAR="$(ls "$ROOT"/target/TestingBotTunnel-*-shaded.jar 2>/dev/null | head -1)"
-[ -z "$JAR" ] && { echo "No shaded jar found. Run: mvn package"; exit 1; }
+# The newest shaded jar, and only when it is unambiguous. `ls | head -1` took the
+# alphabetically first, so after a version bump without `mvn clean` it silently picked
+# the *older* artifact -- 5.10 sorts before 5.9, and the run then verified the previous
+# release while reporting the new one.
+select_shaded_jar() {
+  local root="$1"
+  local jars=()
+  while IFS= read -r line; do jars+=("$line"); done < <(
+    ls -t "$root"/target/TestingBotTunnel-*-shaded.jar 2>/dev/null
+  )
+  if [ "${#jars[@]}" -eq 0 ]; then
+    return 1
+  fi
+  if [ "${#jars[@]}" -gt 1 ]; then
+    echo "Warning: several shaded jars in target/; using the newest ($(basename "${jars[0]}"))." >&2
+    echo "         Run 'mvn clean package' if that is not what you meant." >&2
+  fi
+  printf '%s\n' "${jars[0]}"
+}
+
+JAR="$(select_shaded_jar "$ROOT")" || { echo "No shaded jar found. Run: mvn package"; exit 1; }
 
 WORK="$(mktemp -d "${TMPDIR:-/tmp}/tb-e2e.XXXXXX")"
 MARKER="TB-E2E-$(date +%s)-$$"