From 307970cf12321cccdec894b6a8adc132638c6db6 Mon Sep 17 00:00:00 2001 From: Kevin O'Donnell Date: Wed, 6 May 2026 14:01:32 -0400 Subject: [PATCH 1/7] fix(log): make level_decrease safe under set -e (SUR-2490) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Use arithmetic assignment so decrementing 1→0 does not trip (( ))'s zero status. Clarify @doc for the LOG_LEVEL=0 path and warning gating (SUR-2476). test(log): level_decrease coverage for set -e and floor (SUR-2476 SUR-2490) Add set -e subshell regression, optional warning visibility when enabled, and bats_require_minimum_version for run --separate-stderr. --- bash/log.sh | 11 +++++++---- tests/log.bats | 36 +++++++++++++++++++++++++++++++++--- 2 files changed, 40 insertions(+), 7 deletions(-) diff --git a/bash/log.sh b/bash/log.sh index 9b604e5..996db5b 100644 --- a/bash/log.sh +++ b/bash/log.sh @@ -117,14 +117,17 @@ function log::level_increase() { } function log::level_decrease() { - @doc Decrease the LOG_LEVEL by 1. The minimum effective level is 0, calling this - @doc function at LOG_LEVEL=0 logs a warning and returns without changing the level. - @doc Mirrors log::level_increase. Call log::level after adjusting to re-apply flags. + @doc Decrease LOG_LEVEL by 1 and refresh disable flags via log::level. At \ + LOG_LEVEL=0, calls log::warn and returns unchanged — the message is gated \ + like other warnings, so at default level 0 it may not print. Safe under \ + set -e. Pair with log::level_increase for reversible verbosity toggles. if ((LOG_LEVEL <= 0)); then log::warn "log::level_decrease: LOG_LEVEL already at minimum (0); ignoring" return fi - ((LOG_LEVEL -= 1)) + # Use $(( )) not ((var -= 1)): the latter exits 1 when the result is 0, so + # set -e aborts after decrementing from LOG_LEVEL=1 (SUR-2490). + LOG_LEVEL=$((LOG_LEVEL - 1)) log::level "$LOG_LEVEL" } diff --git a/tests/log.bats b/tests/log.bats index a32cf57..21c88f3 100644 --- a/tests/log.bats +++ b/tests/log.bats @@ -1,6 +1,8 @@ #!/usr/bin/env bats # SUR-1850 seed: log::level cumulative gating per documented table. +bats_require_minimum_version 1.5.0 + setup() { load 'helpers.bash' helpers::isolate_home @@ -193,9 +195,9 @@ probe_level() { [[ "$out" == "INFO date=%DATE" ]] } -# SUR-2475: log::level_decrease floor guard +# SUR-2490 / SUR-2476: log::level_decrease under set -e and floor guard -@test "log::level_decrease from non-zero decrements LOG_LEVEL (SUR-2475)" { +@test "log::level_decrease from non-zero decrements LOG_LEVEL (SUR-2476)" { run bash -c " LOG_LEVEL=2 source '$REPO_ROOT/bash/includer.sh' @@ -207,7 +209,21 @@ probe_level() { [ "$output" = "1" ] } -@test "log::level_decrease at LOG_LEVEL=0 leaves level unchanged (SUR-2475)" { +@test "log::level_decrease completes under set -e when LOG_LEVEL > 0 (SUR-2490)" { + run bash -c " + set -e + LOG_LEVEL=1 + LOGFILE_DISABLE=true + source '$REPO_ROOT/bash/includer.sh' + @include log + log::level_decrease + echo \"ok LOG_LEVEL=\$LOG_LEVEL\" + " + [ "$status" -eq 0 ] + [[ "$output" == *"ok LOG_LEVEL=0"* ]] +} + +@test "log::level_decrease at LOG_LEVEL=0 leaves level unchanged (SUR-2476)" { run bash -c " LOG_LEVEL=0 source '$REPO_ROOT/bash/includer.sh' @@ -218,3 +234,17 @@ probe_level() { [ "$status" -eq 0 ] [ "$output" = "0" ] } + +@test "log::level_decrease at floor emits log::warn when warnings enabled (SUR-2476)" { + run --separate-stderr bash -c " + LOG_LEVEL=0 + LOG_DISABLE_WARNING=false + source '$REPO_ROOT/bash/includer.sh' + @include log + log::level_decrease + echo \$LOG_LEVEL + " + [ "$status" -eq 0 ] + [ "$output" = "0" ] + [[ "$stderr" == *"LOG_LEVEL already at minimum"* ]] +} From e5759089e1fb247aaaab6eb9ee203464084eeeef Mon Sep 17 00:00:00 2001 From: Kevin O'Donnell Date: Wed, 6 May 2026 14:05:34 -0400 Subject: [PATCH 2/7] fix(update-repo-tags): validate prerel before lightweight bump (SUR-2457) Reject non-integer prerel segments with a logged notice and reset the next counter to 1. Use NEXT_PREREL consistently in the semver bump and pN rewrite. Tests cover v0.1.0-foo and v0.1.0-rc.1 paths. --- bash/update-repo-tags | 16 +++++++++++++--- tests/update-repo-tags.bats | 22 ++++++++++++++++++++++ 2 files changed, 35 insertions(+), 3 deletions(-) diff --git a/bash/update-repo-tags b/bash/update-repo-tags index 18ae75b..60e895d 100755 --- a/bash/update-repo-tags +++ b/bash/update-repo-tags @@ -81,9 +81,19 @@ elif [ "$CHANGES" -gt 0 ]; then # if we can't annotate tag then we must do a prerel bump if [ "${ANNOTATE_TAG}" = "false" ]; then PREREL=$("$(dirs::of)/semver" get prerel "$LATEST_VERSION") - ((PREREL++)) - TAG=$("$(dirs::of)/semver" bump prerel "$PREREL" "$LATEST_VERSION") - TAG=${TAG/"-$PREREL"/"p$PREREL"} + # Lightweight-tag path: numeric prerel segments only. Non-numeric values + # break arithmetic and the -$PREREL substitution can corrupt the tag + # (SUR-2457). Empty prerel starts the pN sequence at 1. + if [[ -z "$PREREL" ]]; then + NEXT_PREREL=1 + elif [[ "$PREREL" =~ ^[0-9]+$ ]]; then + NEXT_PREREL=$((10#$PREREL + 1)) + else + log::notice "Non-numeric prerel '${PREREL}' in ${LATEST_VERSION}; resetting lightweight prerel counter to 1" + NEXT_PREREL=1 + fi + TAG=$("$(dirs::of)/semver" bump prerel "$NEXT_PREREL" "$LATEST_VERSION") + TAG=${TAG/"-${NEXT_PREREL}"/"p${NEXT_PREREL}"} else TAG=$("$(dirs::of)/semver" bump patch "$LATEST_VERSION") fi diff --git a/tests/update-repo-tags.bats b/tests/update-repo-tags.bats index 6905e47..7fb6f2c 100644 --- a/tests/update-repo-tags.bats +++ b/tests/update-repo-tags.bats @@ -3,6 +3,8 @@ # against a synthetic git repo. Specifically locks down the Xp1 prerel # post-processing on lines 84-86 of update-repo-tags (replace -N with pN). +bats_require_minimum_version 1.5.0 + setup() { load 'helpers.bash' helpers::isolate_home @@ -96,6 +98,26 @@ latest_tag() { [ "$(latest_tag)" = "v0.1.0p2" ] } +@test "lightweight tag with non-numeric prerel resets to p1 with notice (SUR-2457)" { + commit_msg "feat: initial" + light_tag v0.1.0-foo + commit_msg "fix: something" + run --separate-stderr "$UPDATE" -t "$REPO" + [ "$status" -eq 0 ] + [ "$(latest_tag)" = "v0.1.0p1" ] + [[ "$stderr" == *"Non-numeric prerel"* ]] +} + +@test "lightweight tag with dotted prerel does not corrupt next tag (SUR-2457)" { + commit_msg "feat: initial" + light_tag v0.1.0-rc.1 + commit_msg "fix: something" + run --separate-stderr "$UPDATE" -t "$REPO" + [ "$status" -eq 0 ] + [ "$(latest_tag)" = "v0.1.0p1" ] + [[ "$stderr" == *"Non-numeric prerel"* ]] +} + @test "refuses to bump minor on breaking change without -b" { commit_msg "feat: initial" annotated_tag v0.1.0 From 51be19b16f38c12b8711eeed7285852c5fdb37a0 Mon Sep 17 00:00:00 2001 From: Kevin O'Donnell Date: Wed, 6 May 2026 14:07:28 -0400 Subject: [PATCH 3/7] docs(options): document NO_SYNTAX_EXIT and parse_available (SUR-2473) Extend @doc for options::parse and options::parse_available. Avoid semicolons and parentheses inside @doc continuations so prose is not executed as shell. docs(includer): explain @include and includer::find (SUR-2474) Add a block comment for resolution, cksum deduplication, and return-1 semantics. Extend includer::find @doc without shell metacharacters. --- bash/includer.sh | 17 ++++++++++++++++- bash/options.sh | 12 ++++++++++-- 2 files changed, 26 insertions(+), 3 deletions(-) diff --git a/bash/includer.sh b/bash/includer.sh index f3da3a0..1e2a351 100644 --- a/bash/includer.sh +++ b/bash/includer.sh @@ -19,6 +19,19 @@ source "$(dirname "${BASH_SOURCE[0]}")/doc.sh" @package includer +# @include NAME — load bash/NAME or bash/NAME.sh once per process. +# +# Resolution: try dirname(BASH_SOURCE)/NAME first; if unreadable, try the +# same path with a .sh suffix. Same order as includer::find. +# +# Deduplication: cksum(true_file) defines a global guard include_; +# the first successful source sets it so later @include of the same path +# is a no-op. Two paths that cksum differently (e.g. symlink vs canonical) +# load twice; hardlinks to the same inode share a cksum and dedupe. +# +# Errors: missing file prints to stderr and returns 1 (does not exit), +# so callers can handle failures under set -e. + function @include { local include_file=${1:?} local true_file @@ -40,7 +53,9 @@ function @include { } function includer::find { - @doc Find the file referred to as an include. + @doc Resolve bash/STEM or bash/STEM.sh the same way as @include: bare \ + filename first, .sh suffix second. Echo the readable path or return 1 \ + when missing. @arg _1_ the name of the include local include_file=${1:?} local true_file diff --git a/bash/options.sh b/bash/options.sh index 20132f6..0a36ae0 100644 --- a/bash/options.sh +++ b/bash/options.sh @@ -263,7 +263,10 @@ function options::standard() { } function options::parse_available() { - @doc parse the options using the provided argument array + @doc Parse argv against the configured option set. Does not exit when argv \ + is empty. Callers that need the legacy "bare invocation prints help" \ + behaviour should use options::parse or set NO_SYNTAX_EXIT around \ + options::parse. Always runs mandatory-option validation after getopts. @arg "$@" the provided argument array OPTIONS_SEEN=() while options::getopts opt "$@"; do @@ -306,7 +309,12 @@ function options::parse_available() { } function options::parse() { - @doc parse the options using the provided argument array, if no args passes print syntax and exit + @doc Parse argv against the configured option set. After a successful \ + parse, if argv was empty so no flags were consumed and OPTIND is still 1 \ + and NO_SYNTAX_EXIT is unset, prints syntax and exits 1. Set NO_SYNTAX_EXIT \ + to any non-empty value to allow zero-flag invocations such as commands \ + that take only positionals. That leaks the opt-ind state from this \ + parse into later code, so clear or reset OPTIND if you parse again. @arg "$@" the provided argument array options::parse_available "$@" if [ -z "${NO_SYNTAX_EXIT}" ] && [ "${OPTIND}" -eq 1 ]; then From ca1cb562f8bc50cccc537099ecbfc00f643156ad Mon Sep 17 00:00:00 2001 From: Kevin O'Donnell Date: Wed, 6 May 2026 14:11:15 -0400 Subject: [PATCH 4/7] test(pagerduty): incident_key JSON and curl --fail-with-body (SUR-2479) Assert jq shape for empty vs non-empty incident_key and verify the HTTP path passes --fail-with-body through the curl wrapper on failure. --- tests/pagerduty.bats | 36 ++++++++++++++++++++++++++++++++++++ 1 file changed, 36 insertions(+) diff --git a/tests/pagerduty.bats b/tests/pagerduty.bats index f11ead5..8b9b7ce 100644 --- a/tests/pagerduty.bats +++ b/tests/pagerduty.bats @@ -103,6 +103,42 @@ setup() { [[ "$output" == *"key-1"* ]] } +@test "pagerduty::_incident_data omits incident_key when key is empty (SUR-2479)" { + run bash -c " + source '$REPO_ROOT/bash/includer.sh' + @include pagerduty + pagerduty::_incident_data svc incident title '' | jq -e '.incident | has(\"incident_key\") | not' + " + [ "$status" -eq 0 ] +} + +@test "pagerduty::_incident_data includes incident_key when key non-empty (SUR-2479)" { + run bash -c " + source '$REPO_ROOT/bash/includer.sh' + @include pagerduty + pagerduty::_incident_data svc incident title my-key | jq -e '.incident.incident_key == \"my-key\"' + " + [ "$status" -eq 0 ] +} + +@test "pagerduty::send_incident passes --fail-with-body to curl on HTTP failure path (SUR-2479)" { + run bash -c " + source '$REPO_ROOT/bash/includer.sh' + @include pagerduty + pagerduty::_curl() { + case \" \$* \" in + *' --fail-with-body '*) ;; + *) echo 'missing --fail-with-body' >&2; return 99 ;; + esac + return 22 + } + pagerduty::send_incident SVC1 incident T from token '' + " + [ "$status" -eq 22 ] + [[ "$output" != *"missing --fail-with-body"* ]] + [[ "$output" == *"PagerDuty incident send failed"* ]] +} + @test "pagerduty::send_incident does not emit Token token= secret in xtrace (SUR-2339)" { run bash -c " trace=\$(mktemp) From cd24948cf67d6c6b91b822f796b214ceba5061cb Mon Sep 17 00:00:00 2001 From: Kevin O'Donnell Date: Wed, 6 May 2026 14:33:34 -0400 Subject: [PATCH 5/7] fix(tests): silence shellcheck SC2154 for bats run --separate-stderr Declare local stderr/stderr_lines before run so shellcheck accepts $stderr populated by bats, matching vendored bats test patterns. fix(tests): remove unused stderr_lines locals for shellcheck SC2034 failed CI: stderr_lines was declared with local but never read. Assertions use $stderr only from run --separate-stderr. --- tests/log.bats | 1 + tests/update-repo-tags.bats | 2 ++ 2 files changed, 3 insertions(+) diff --git a/tests/log.bats b/tests/log.bats index 21c88f3..ae24a75 100644 --- a/tests/log.bats +++ b/tests/log.bats @@ -236,6 +236,7 @@ probe_level() { } @test "log::level_decrease at floor emits log::warn when warnings enabled (SUR-2476)" { + local stderr run --separate-stderr bash -c " LOG_LEVEL=0 LOG_DISABLE_WARNING=false diff --git a/tests/update-repo-tags.bats b/tests/update-repo-tags.bats index 7fb6f2c..b012658 100644 --- a/tests/update-repo-tags.bats +++ b/tests/update-repo-tags.bats @@ -99,6 +99,7 @@ latest_tag() { } @test "lightweight tag with non-numeric prerel resets to p1 with notice (SUR-2457)" { + local stderr commit_msg "feat: initial" light_tag v0.1.0-foo commit_msg "fix: something" @@ -109,6 +110,7 @@ latest_tag() { } @test "lightweight tag with dotted prerel does not corrupt next tag (SUR-2457)" { + local stderr commit_msg "feat: initial" light_tag v0.1.0-rc.1 commit_msg "fix: something" From dfba72a22f7fd08c7bc73a1aeab8a812e0e5d113 Mon Sep 17 00:00:00 2001 From: Kevin O'Donnell Date: Wed, 6 May 2026 15:04:49 -0400 Subject: [PATCH 6/7] fix(tests): remove on-change bats race with start_version snapshot Defer the marker mutation until after slow CI can take the initial find|cksum baseline; widen the outer timeout so the first run still completes under throttled stdin polling. --- tests/on-change.bats | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/on-change.bats b/tests/on-change.bats index bd8b334..991471f 100644 --- a/tests/on-change.bats +++ b/tests/on-change.bats @@ -43,10 +43,10 @@ printf '%s\n' "\$@" >>"$argv_log" EOF chmod +x "$stub_bin/record_argv" ( - sleep 0.4 + sleep 2 echo trigger >>"$watch_dir/marker" ) & - run env PATH="$stub_bin:$PATH" timeout 8s "$ONCHANGE" -W "$watch_dir" -t 1 -v -v -- "$stub_bin/record_argv" ok + run env PATH="$stub_bin:$PATH" timeout 15s "$ONCHANGE" -W "$watch_dir" -t 1 -v -v -- "$stub_bin/record_argv" ok [[ "$output" == *"Working directory $watch_dir"* ]] [[ "$output" == *"Watching directory $watch_dir"* ]] [[ "$output" == *"Polling interval 1"* ]] @@ -66,10 +66,10 @@ printf '%s\n' "\$@" >>"$argv_log" EOF chmod +x "$stub_bin/record_argv" ( - sleep 0.4 + sleep 2 echo trigger >>"$watch_dir/marker" ) & - run env PATH="$stub_bin:$PATH" timeout 8s "$ONCHANGE" -W "$watch_dir" -w "$watch_dir" -t 1 -- "$stub_bin/record_argv" "arg one" "arg two" + run env PATH="$stub_bin:$PATH" timeout 15s "$ONCHANGE" -W "$watch_dir" -w "$watch_dir" -t 1 -- "$stub_bin/record_argv" "arg one" "arg two" grep -qx 'arg one' "$argv_log" grep -qx 'arg two' "$argv_log" rm -rf "$watch_dir" "$stub_bin" From 298dfcb02f4b3f1a671ca5d1ba018498780fddb9 Mon Sep 17 00:00:00 2001 From: Kevin O'Donnell Date: Wed, 6 May 2026 15:28:47 -0400 Subject: [PATCH 7/7] fix: correct parse_available docs and set -e-safe level_increase - options::parse_available @doc: empty-argv help exit is options::parse only; NO_SYNTAX_EXIT applies to options::parse for zero-flag parses, with OPTIND caveat. Avoid semicolon inside continued @doc (would run as a separate command). - log::level_increase: use $((LOG_LEVEL + 1)) like level_decrease (SUR-2490). - tests: bats case for LOG_LEVEL=-1 under set -e. --- bash/log.sh | 4 +++- bash/options.sh | 8 +++++--- tests/log.bats | 14 ++++++++++++++ 3 files changed, 22 insertions(+), 4 deletions(-) diff --git a/bash/log.sh b/bash/log.sh index 996db5b..85cc5e8 100644 --- a/bash/log.sh +++ b/bash/log.sh @@ -112,7 +112,9 @@ unset __log_value_trace __log_value_debug __log_value_info __log_value_warning function log::level_increase() { @doc Increase the LOG_LEVEL - ((LOG_LEVEL += 1)) + # Use $(( )) not ((var += 1)): the latter exits 1 when the result is 0, so + # set -e aborts (symmetry with log::level_decrease, SUR-2490). + LOG_LEVEL=$((LOG_LEVEL + 1)) log::level "$LOG_LEVEL" } diff --git a/bash/options.sh b/bash/options.sh index 0a36ae0..cb5cd65 100644 --- a/bash/options.sh +++ b/bash/options.sh @@ -264,9 +264,11 @@ function options::standard() { function options::parse_available() { @doc Parse argv against the configured option set. Does not exit when argv \ - is empty. Callers that need the legacy "bare invocation prints help" \ - behaviour should use options::parse or set NO_SYNTAX_EXIT around \ - options::parse. Always runs mandatory-option validation after getopts. + is empty. Callers that want empty argv to print syntax help and exit should \ + use options::parse. Setting NO_SYNTAX_EXIT is only for use with \ + options::parse: it suppresses that empty-argv syntax exit so zero-flag \ + invocations are allowed. See options::parse for the OPTIND caveat when \ + parsing again. Always runs mandatory-option validation after getopts. @arg "$@" the provided argument array OPTIONS_SEEN=() while options::getopts opt "$@"; do diff --git a/tests/log.bats b/tests/log.bats index ae24a75..c781637 100644 --- a/tests/log.bats +++ b/tests/log.bats @@ -249,3 +249,17 @@ probe_level() { [ "$output" = "0" ] [[ "$stderr" == *"LOG_LEVEL already at minimum"* ]] } + +@test "log::level_increase completes under set -e when result is 0 (SUR-2490 parity)" { + run bash -c " + set -e + LOG_LEVEL=-1 + LOGFILE_DISABLE=true + source '$REPO_ROOT/bash/includer.sh' + @include log + log::level_increase + echo \"ok LOG_LEVEL=\$LOG_LEVEL\" + " + [ "$status" -eq 0 ] + [[ "$output" == *"ok LOG_LEVEL=0"* ]] +}