From 4d4d1dd5d578eb71f58764eed35e0a8703111682 Mon Sep 17 00:00:00 2001 From: nexxo Date: Wed, 29 Jul 2026 00:00:17 +0200 Subject: [PATCH] Stop the update agent dying on its own tag arithmetic MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Since 1.2.0 the agent has ended with a non-zero status on every tick, before writing anything. The console showed a check that never finished and a last-checked time frozen at the moment of the deployment, and the update button could not be reached at all — the one mechanism that would have delivered the fix. Two instances of the same trap, both mine, both under `set -Eeuo pipefail`: The tag count was `release_version_gt … && echo`. The LAST iteration is almost always a tag that is not newer, so the loop ended non-zero, pipefail carried it out of the pipeline, the command substitution inherited it, and set -e ended the agent. `if` rather than `&&` is the whole fix: an if whose condition is false still returns 0. The newest tag came from `git tag … | head -1`. head exits after one line, git takes SIGPIPE, pipefail does the rest. install-agent.sh has carried a written warning about exactly this since it was written; I read it and wrote it anyway. Both now live in deploy/lib/release.sh, because the reason neither was caught is that no PHP test can reach a bash pipeline. tests/Feature/ReleaseComparisonTest.php runs the real functions in a real throwaway repository under the agent's own set -Eeuo pipefail, and asserts the EXIT CODE as well as the answer — an answer nobody lives to read is not an answer. The SIGPIPE case needs four hundred tags to reproduce, because a short list finishes writing before head leaves. Co-Authored-By: Claude Opus 5 --- VERSION | 2 +- deploy/lib/release.sh | 52 +++++++++ deploy/update-agent.sh | 22 ++-- tests/Feature/ReleaseComparisonTest.php | 145 ++++++++++++++++++++++++ 4 files changed, 208 insertions(+), 13 deletions(-) create mode 100644 tests/Feature/ReleaseComparisonTest.php diff --git a/VERSION b/VERSION index 6085e94..23aa839 100644 --- a/VERSION +++ b/VERSION @@ -1 +1 @@ -1.2.1 +1.2.2 diff --git a/deploy/lib/release.sh b/deploy/lib/release.sh index d351c10..ef53480 100644 --- a/deploy/lib/release.sh +++ b/deploy/lib/release.sh @@ -42,6 +42,58 @@ release_manifest_version() { sed -n 's/.*"version"[[:space:]]*:[[:space:]]*"\([^"]*\)".*/\1/p' "$MANIFEST_FILE" 2>/dev/null | head -1 } +# release_version_gt A B — true when A is strictly a higher version than B. +# +# By version, not by ancestry: a tag can be cut from anywhere, and what an +# operator is being offered is a version number. `sort -V` does the comparing, +# so v1.10.0 correctly beats v1.9.0 where a string comparison would not. +release_version_gt() { + [[ "$1" != "$2" ]] && [[ "$(printf '%s\n%s\n' "$1" "$2" | sort -V | tail -1)" == "$1" ]] +} + +# release_newest_tag — the highest v* tag by version order, or nothing. +# +# Deliberately not `git tag … | head -1`. head exits after the first line, git +# gets SIGPIPE, and under `set -o pipefail` that non-zero status leaves the +# command substitution and ends the caller — the same way the tag count did, +# and the same trap install-agent.sh already carries a warning about. A read +# loop over a process substitution is not a pipeline and cannot do it. +release_newest_tag() { + local tag + while read -r tag; do + [[ -n "$tag" ]] || continue + printf '%s' "$tag" + return 0 + done < <(git tag --list 'v*' --sort=-v:refname 2>/dev/null) + + return 0 +} + +# release_tags_ahead VERSION — how many v* tags are higher than VERSION. +# +# Lives here rather than inline in the agent because it has to be testable. The +# inline version killed the update agent on every tick: the loop body was +# `release_version_gt … && echo`, so the LAST iteration — almost always a tag +# that is not newer — left the loop with a non-zero status, `pipefail` carried +# it out of the pipeline, the command substitution inherited it, and `set -e` +# ended the agent before it could write a status. The console then showed a +# check that never finished and a last-checked time frozen at the deployment. +# +# `if` rather than `&&` is the whole fix: an if whose condition is false still +# returns 0. Anything added here must keep that property. +release_tags_ahead() { + local current="$1" tag count=0 + + while read -r tag; do + [[ -n "$tag" ]] || continue + if release_version_gt "${tag#v}" "$current"; then + count=$((count + 1)) + fi + done < <(git tag --list 'v*' --sort=-v:refname 2>/dev/null) + + printf '%s' "$count" +} + release_remember() { # release_remember MODE SOURCE mkdir -p "$(dirname "$MODE_FILE")" printf '%s' "$1" > "$MODE_FILE" diff --git a/deploy/update-agent.sh b/deploy/update-agent.sh index e117039..70ba34e 100755 --- a/deploy/update-agent.sh +++ b/deploy/update-agent.sh @@ -216,22 +216,20 @@ FETCH_ERROR='' DEPLOYED_VERSION="$(release_manifest_version)" [[ -n "$DEPLOYED_VERSION" ]] || DEPLOYED_VERSION="$(release_version)" -# version_gt A B — true when A is strictly a higher version than B. -version_gt() { - [[ "$1" != "$2" ]] && [[ "$(printf '%s\n%s\n' "$1" "$2" | sort -V | tail -1)" == "$1" ]] -} - if git fetch --quiet --tags --force origin 2>/dev/null; then - # Newest by version order, not by tag date. - NEWEST_TAG="$(git tag --list 'v*' --sort=-v:refname 2>/dev/null | head -1)" + # Newest by version order, not by tag date. Through the helper rather than + # `| head -1`: head exits after one line, git takes SIGPIPE, and pipefail + # ends the agent — see release_newest_tag. + NEWEST_TAG="$(release_newest_tag)" NEWEST_VERSION="${NEWEST_TAG#v}" - if [[ -n "$NEWEST_TAG" ]] && version_gt "$NEWEST_VERSION" "$DEPLOYED_VERSION"; then + if [[ -n "$NEWEST_TAG" ]] && release_version_gt "$NEWEST_VERSION" "$DEPLOYED_VERSION"; then # How many releases ahead, so the console can say "2 Aktualisierungen" - # rather than only that something exists. - BEHIND="$(git tag --list 'v*' --sort=-v:refname 2>/dev/null | while read -r t; do - version_gt "${t#v}" "$DEPLOYED_VERSION" && echo "$t" - done | wc -l | tr -d ' ')" + # rather than only that something exists. Both helpers live in + # deploy/lib/release.sh so a test can run them — the version that lived + # here ended the agent on every tick under `set -e`, and no test in a + # PHP suite can reach a bash pipeline. + BEHIND="$(release_tags_ahead "$DEPLOYED_VERSION")" TARGET_RELEASE="$NEWEST_TAG" REMOTE_COMMIT="$(git rev-parse "refs/tags/${NEWEST_TAG}^{commit}" 2>/dev/null || echo '')" else diff --git a/tests/Feature/ReleaseComparisonTest.php b/tests/Feature/ReleaseComparisonTest.php new file mode 100644 index 0000000..1634ddd --- /dev/null +++ b/tests/Feature/ReleaseComparisonTest.php @@ -0,0 +1,145 @@ +run(); + + (new Process(['rm', '-rf', $repo]))->run(); + + return $process; +} + +it('counts the releases ahead without taking the agent down with it', function () { + // The exact shape that failed: the newest tag is ahead, and the tags after + // it in the list are not — so the loop's final iteration is a false one. + $process = runRelease( + 'release_tags_ahead 1.2.0', + ['v1.1.0', 'v1.1.1', 'v1.2.0', 'v1.2.1'], + ); + + expect($process->getExitCode())->toBe(0) + ->and(trim($process->getOutput()))->toBe('1'); +}); + +it('survives a run where no tag at all is ahead', function () { + // Every iteration false, including the last. This is the ordinary state of + // an up-to-date server, which is to say: most ticks, on most machines. + $process = runRelease( + 'release_tags_ahead 1.2.1', + ['v1.1.0', 'v1.2.0', 'v1.2.1'], + ); + + expect($process->getExitCode())->toBe(0) + ->and(trim($process->getOutput()))->toBe('0'); +}); + +it('survives a repository with no tags whatsoever', function () { + $process = runRelease('release_tags_ahead 1.0.0'); + + expect($process->getExitCode())->toBe(0) + ->and(trim($process->getOutput()))->toBe('0'); +}); + +it('counts several releases ahead', function () { + $process = runRelease( + 'release_tags_ahead 1.1.0', + ['v1.1.0', 'v1.1.1', 'v1.2.0', 'v1.2.1'], + ); + + expect($process->getExitCode())->toBe(0) + ->and(trim($process->getOutput()))->toBe('3'); +}); + +it('picks the newest tag out of a long list without dying on a broken pipe', function () { + // The second instance of the same trap. `git tag … | head -1` makes head + // exit after one line, git takes SIGPIPE, and pipefail ends the caller. + // Few tags hide it — git finishes writing before head leaves — so it needs + // enough of them to actually fill a pipe buffer. + $tags = []; + + for ($i = 1; $i <= 400; $i++) { + $tags[] = 'v1.'.$i.'.0'; + } + + $process = runRelease('release_newest_tag', $tags); + + expect($process->getExitCode())->toBe(0) + ->and(trim($process->getOutput()))->toBe('v1.400.0'); +}); + +it('answers with nothing, and cleanly, when there are no releases yet', function () { + $process = runRelease('release_newest_tag; echo "[$?]"'); + + expect($process->getExitCode())->toBe(0) + ->and(trim($process->getOutput()))->toBe('[0]'); +}); + +it('orders by version, not alphabetically', function () { + // v1.10.0 is newer than v1.9.0 and sorts earlier as a string. Getting this + // wrong means a server sitting on 1.9.0 is told it is current. + $process = runRelease( + 'release_tags_ahead 1.9.0', + ['v1.9.0', 'v1.10.0'], + ); + + expect($process->getExitCode())->toBe(0) + ->and(trim($process->getOutput()))->toBe('1'); +}); + +it('answers the pairwise comparison the same way, and exits cleanly either way', function () { + $cases = [ + ['1.2.1', '1.2.0', 'ja'], + ['1.2.0', '1.2.1', 'nein'], + ['1.2.0', '1.2.0', 'nein'], + ['1.10.0', '1.9.0', 'ja'], + ['2.0.0', '1.99.99', 'ja'], + ]; + + foreach ($cases as [$a, $b, $expected]) { + $process = runRelease("if release_version_gt {$a} {$b}; then echo ja; else echo nein; fi"); + + expect($process->getExitCode())->toBe(0) + ->and(trim($process->getOutput()))->toBe($expected, "{$a} > {$b}"); + } +});