diff --git a/CHANGELOG.md b/CHANGELOG.md index 5b3e3b8d..2c2c2c4a 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,10 @@ # Changelog +### Skills + +- **`run-tests.sh` discovered shell suites only — 32 Python tests never ran in CI** — the `test_*.sh` glob silently orphaned `test_compute_lift.py` (22 tests) and `test_stamp_changelog.py` (10 tests); both passed when run by hand, so the modules looked covered while `stamp-changelog.py` sat untested in the publish path. Discovery now matches `test_*.{sh,py}` and dispatches by extension (both suite kinds self-drive via the entry-point guard `rules/file-hygiene.md` requires). Suite count 17 → 20 (18 shell + 2 Python, including this PR's new baseline suite). Full motivation: PR #183. +- **`release`: registry baseline was a pasted pipeline with no hardening** — `SKILL.md` had the operator paste `tessl plugin info | grep | awk`, sharing its parse with `verify-publish-landed.sh` but none of its guards: a tessl warning on stderr could poison the parse, and a parse miss silently yielded an empty `PRE` that satisfied the release contract's conjunct 2 vacuously (every version "advances past" empty), confirming an unpublished release. Extracted to `skills/release/capture-registry-baseline.sh`, which emits `{"version": ""}` or exits 2 — never an empty baseline, and never a shape `verify-publish-landed.sh`'s comparator cannot order. Full motivation: PR #183. + ## 0.3.93 — 2026-07-17 ### Skills diff --git a/evals/pr-merge-and-post-merge-cleanup/criteria.json b/evals/pr-merge-and-post-merge-cleanup/criteria.json index ea479887..652e30f0 100644 --- a/evals/pr-merge-and-post-merge-cleanup/criteria.json +++ b/evals/pr-merge-and-post-merge-cleanup/criteria.json @@ -45,7 +45,7 @@ { "name": "Pre-merge registry baseline captured", "max_score": 8, - "description": "Captures the registry's current Latest Version BEFORE invoking `gh pr merge` (e.g., `PRE=$(tessl tile info / | grep 'Latest Version' | awk '{print $NF}')`) so the post-merge check has a baseline to compare against. Scripts that skip the baseline and only inspect post-merge registry state score zero — without the baseline, the agent can't distinguish 'this release published' from 'a prior release already shipped this version'" + "description": "Captures the registry's current Latest Version BEFORE invoking `gh pr merge`, using the release skill's guarded baseline-capture step rather than a raw hand-parse of `tessl plugin info`, so the post-merge check has a reliable baseline. Scripts that skip the baseline entirely score zero — without it the agent can't distinguish 'this release published' from 'a prior release already shipped this version'. A raw pipeline that silently yields an empty baseline on a parse miss scores partial at most: the graded manner is capturing the pre-merge value through the guarded step the skill prescribes" }, { "name": "SHA-bound publish-run resolution", diff --git a/scripts/run-tests.sh b/scripts/run-tests.sh index ff0f31ad..74ecc6b7 100755 --- a/scripts/run-tests.sh +++ b/scripts/run-tests.sh @@ -1,5 +1,5 @@ #!/usr/bin/env bash -# Discover and run every module's bash unit-test suite, in one pass. +# Discover and run every module's unit-test suite, in one pass. # # rules/testing-standards.md mandates that every module ship tests, but # nothing executed them: the publish pipeline ran lint + skill-review + @@ -8,8 +8,15 @@ # on every PR (tests.yml) and again before publish (publish.yml), so a # failing suite blocks the merge and the release. # -# Discovers /**/tests/test_*.sh, runs each in its own subshell so -# one suite's `set`/`cd`/traps can't leak into the next. +# Discovers /**/tests/test_*.sh and /**/tests/test_*.py, and +# runs each in its own process (`bash file` / `python3 file`) so one suite's +# `set`/`cd`/traps/interpreter state can't leak into the next. Discovery +# covers BOTH languages deliberately: a `.sh`-only +# glob silently orphaned every Python suite in the repo — they looked +# covered while never executing in CI, including one wired into the publish +# path. Each suite self-drives (a shell harness prints its own summary and +# exits non-zero; a unittest module does the same via its entry-point +# guard), so dispatch is by extension and nothing else. # # Output contract (rules/script-delegation.md — structured stdout): # stdout: one JSON object, the run summary — @@ -26,6 +33,55 @@ set -euo pipefail +# Dispatch a suite to its interpreter by extension. Both suite kinds are +# self-driving — a shell harness counts its own failures and exits +# non-zero; a unittest module does the same through the entry-point guard +# `rules/file-hygiene.md` requires — so the runner only picks the +# interpreter and reads the exit code. +# +# Interpreters are overridable so a runner with a differently-named binary +# (python3.11, a venv shim) can point at it without editing this file, and +# so the missing-interpreter path is testable without breaking PATH for the +# harness itself. +SH_BIN="${SH_BIN:-bash}" +PY_BIN="${PY_BIN:-python3}" + +# A dispatcher fault must be distinguishable from a suite's own exit code. +# No exit code can do that job: 2 collides with the harnesses here (they +# exit 2 on a fatal precondition), and any reserved sentinel — 125 included +# — is only a soft invariant, since nothing stops a suite from returning it +# and being misreported as a broken runner. So the DISPATCH FAULT does not +# travel by exit code: run_suite sets DISPATCH_FAULT_SUITE for a dispatch +# fault (unknown suite type, missing interpreter). The exit code run_suite +# returns is still the suite's own verdict on the happy path — the caller +# reads it — but it is only trusted as a verdict when the side channel is +# empty. +DISPATCH_FAULT_SUITE="" + +run_suite() { + local suite="$1" interp + case "$suite" in + *.sh) interp="$SH_BIN" ;; + *.py) interp="$PY_BIN" ;; + *) + DISPATCH_FAULT_SUITE="$suite" + echo "run-tests: no interpreter for suite type: $suite" >&2 + return 1 + ;; + esac + # Check the interpreter exists before dispatching. Without this, a runner + # lacking python3 gives the suite exit 127, which the caller counts as a + # FAILING TEST rather than the setup error (rc 2) this contract promises — + # reporting a red suite where the truth is an unprovisioned runner, and + # sending whoever reads CI to debug a test that never ran. + if ! command -v "$interp" >/dev/null; then + DISPATCH_FAULT_SUITE="$suite" + echo "run-tests: '${interp}' not found on PATH — cannot run ${suite}; install it on this runner (this is a setup error, not a test failure)" >&2 + return 1 + fi + "$interp" "$suite" +} + # JSON string escaper. A Unix path may contain any byte except NUL and # '/', so escape the JSON-mandatory characters AND the C0 control set # (newline/tab/CR/etc.) rather than assuming paths are control-free. @@ -72,7 +128,12 @@ main() { # the redirection, and the `if !` catches it — a command substitution # can't be used because bash strips NUL bytes from its output. local tmplist; tmplist="$(mktemp)" - if ! find "$base" -type f -path '*/tests/test_*.sh' -print0 | sort -z > "$tmplist"; then + # `-path '*/tests/test_*.sh'` narrows to a tests/ dir, but `find`'s `*` + # matches `/` too, so it still matches a NESTED file like + # `tests/test_fixtures/helper.sh`. The direct-child requirement can't be + # expressed portably in `-path`, so discovery casts wide here and the + # collection loop below enforces `dirname == */tests`. + if ! find "$base" -type f \( -path '*/tests/test_*.sh' -o -path '*/tests/test_*.py' \) -print0 | sort -z > "$tmplist"; then rm -f "$tmplist" echo "run-tests: suite discovery (find) failed under $base" >&2 printf '{"suites":0,"passed":0,"failed":0,"failures":[],"error":%s}\n' \ @@ -80,27 +141,51 @@ main() { return 2 fi + # A suite is a DIRECT child of a `tests/` directory. Drop anything nested + # deeper (a fixture/helper named test_* under tests/test_fixtures/), which + # the wide `find` match above still lets through. local suites=() local s while IFS= read -r -d '' s; do - [[ -n "$s" ]] && suites+=("$s") + [[ -n "$s" ]] || continue + [[ "$(dirname "$s")" == */tests ]] || continue + suites+=("$s") done < "$tmplist" rm -f "$tmplist" if [[ ${#suites[@]} -eq 0 ]]; then - echo "run-tests: no test suites found under ${base}/**/tests/test_*.sh" >&2 + echo "run-tests: no test suites found under ${base}/**/tests/test_*.{sh,py}" >&2 printf '{"suites":0,"passed":0,"failed":0,"failures":[],"error":%s}\n' \ - "$(json_str "no test suites found under ${base}/**/tests/test_*.sh")" + "$(json_str "no test suites found under ${base}/**/tests/test_*.{sh,py}")" return 2 fi echo "Running ${#suites[@]} test suite(s):" >&2 echo "" >&2 - local failed=() + # A dispatcher fault (interpreter absent, unknown suite type) is NOT a test + # verdict. Folding it into `failed` would report "1 suite failed" for a + # runner that never executed the suite at all — a red CI pointing at + # innocent tests. Surface it as the contract's setup error (rc 2, suites=0) + # and stop: subsequent suites would hit the same missing interpreter and + # bury the cause under noise. + # + # Two signals, read in order: the side channel says whether this was a + # dispatch fault, and only if it wasn't is the exit code trusted as the + # suite's verdict. That ordering is what lets a suite's own 2 or 125 count + # as a failure while a dispatch fault (same numeric codes possible) does + # not — the value never disambiguates them, the side channel does. + local failed=() setup_error="" for s in "${suites[@]}"; do echo "▶ $s" >&2 - if bash "$s" >&2; then + local suite_rc=0 + DISPATCH_FAULT_SUITE="" + run_suite "$s" >&2 || suite_rc=$? + if [[ -n "$DISPATCH_FAULT_SUITE" ]]; then + setup_error="$DISPATCH_FAULT_SUITE" + break + fi + if [[ $suite_rc -eq 0 ]]; then echo " ✓ $s" >&2 else echo " ✗ $s" >&2 @@ -109,6 +194,14 @@ main() { echo "" >&2 done + if [[ -n "$setup_error" ]]; then + echo "─────────────────────────────────────────────" >&2 + echo "SETUP ERROR: cannot run ${setup_error} — see stderr above" >&2 + printf '{"suites":0,"passed":0,"failed":0,"failures":[],"error":%s}\n' \ + "$(json_str "cannot run ${setup_error}: interpreter missing or unknown suite type")" + return 2 + fi + echo "─────────────────────────────────────────────" >&2 if [[ ${#failed[@]} -gt 0 ]]; then echo "FAILED: ${#failed[@]}/${#suites[@]} suite(s):" >&2 diff --git a/scripts/tests/test_run_tests.sh b/scripts/tests/test_run_tests.sh index 05f66742..6e6a8d30 100755 --- a/scripts/tests/test_run_tests.sh +++ b/scripts/tests/test_run_tests.sh @@ -30,6 +30,18 @@ add_suite() { printf '#!/usr/bin/env bash\nexit %s\n' "$exit_code" > "$dir/test_$name.sh" } +# A Python suite, shaped like the real ones: self-driving via the +# entry-point guard rules/file-hygiene.md requires. Discovery matched +# `test_*.sh` only, so every Python suite in the repo was orphaned — +# present, passing when run by hand, never executed by CI. +add_py_suite() { + local base="$1" name="$2" exit_code="$3" + local dir="$base/skills/$name/tests" + mkdir -p "$dir" + printf 'import sys\nif __name__ == "__main__":\n sys.exit(%s)\n' \ + "$exit_code" > "$dir/test_$name.py" +} + # Runs the runner; sets OUT (stdout), ERR (stderr), CODE (exit). invoke() { local base="$1" errf; errf="$(mktemp)" @@ -124,6 +136,123 @@ else fi rm -rf "$base" +# --- a Python suite is discovered and run at all --- +# Pre-fix this exits 2 ("no test suites found"): the .sh-only glob matched +# nothing, so a tree of passing Python tests read as an empty tree. +base="$(make_base)"; add_py_suite "$base" pyalpha 0 +invoke "$base" +if [[ "$CODE" == 0 ]] && [[ "$(jq -r .suites <<<"$OUT")" == 1 ]] \ + && [[ "$(jq -r .passed <<<"$OUT")" == 1 ]]; then + pass "python-only tree -> discovered, exit 0, suites=1" +else + fail "python discovery: code=$CODE out=$OUT" +fi +rm -rf "$base" + +# --- a failing Python suite reddens the run --- +# Discovery alone isn't enough: a suite that runs but whose exit code is +# dropped would count as passed and let a real regression ship green. +base="$(make_base)"; add_py_suite "$base" pyalpha 0; add_py_suite "$base" pydoomed 1 +invoke "$base" +if [[ "$CODE" == 1 ]] && [[ "$(jq -r .failed <<<"$OUT")" == 1 ]] \ + && [[ "$(jq -r '.failures[0]' <<<"$OUT")" == *"test_pydoomed.py" ]]; then + pass "failing python suite -> exit 1, named in failures" +else + fail "python failure: code=$CODE out=$OUT" +fi +rm -rf "$base" + +# --- shell and python suites are counted in one run --- +base="$(make_base)"; add_suite "$base" alpha 0; add_py_suite "$base" pybeta 0 +invoke "$base" +if [[ "$CODE" == 0 ]] && [[ "$(jq -r .suites <<<"$OUT")" == 2 ]] \ + && [[ "$(jq -r .passed <<<"$OUT")" == 2 ]]; then + pass "mixed sh+py tree -> both counted, suites=2" +else + fail "mixed discovery: code=$CODE out=$OUT" +fi +rm -rf "$base" + +# --- a suite that exits 2 is a FAILING SUITE, not a dispatcher fault --- +# The harnesses in this repo exit 2 on a fatal precondition ("fatal: not +# executable"). An earlier draft used 2 as the dispatcher's own setup-error +# code, which made those suites read as a broken runner and stopped the run +# — hiding a real red suite behind a setup error nobody could act on. +base="$(make_base)"; add_suite "$base" alpha 0; add_suite "$base" fatal2 2 +invoke "$base" +if [[ "$CODE" == 1 ]] \ + && [[ "$(jq -r .failed <<<"$OUT")" == 1 ]] \ + && [[ "$(jq -r .suites <<<"$OUT")" == 2 ]] \ + && [[ "$(jq -r '.failures[0]' <<<"$OUT")" == *"test_fatal2.sh" ]]; then + pass "suite exiting 2 -> counted as a failure, not a setup error" +else + fail "suite exit 2: code=$CODE out=$OUT" +fi +rm -rf "$base" + +# --- a python suite exiting 2 is likewise a failure --- +base="$(make_base)"; add_py_suite "$base" pyfatal2 2 +invoke "$base" +if [[ "$CODE" == 1 ]] && [[ "$(jq -r .failed <<<"$OUT")" == 1 ]]; then + pass "python suite exiting 2 -> counted as a failure" +else + fail "python exit 2: code=$CODE out=$OUT" +fi +rm -rf "$base" + +# --- a test_* file NESTED under tests/ is NOT dispatched as a suite --- +# `find`'s `*` matches `/`, so `-path '*/tests/test_*.sh'` alone would match +# `tests/test_fixtures/helper.sh`. The dirname==*/tests filter drops it. +base="$(make_base)"; add_suite "$base" alpha 0 +nested="$base/skills/alpha/tests/test_fixtures"; mkdir -p "$nested" +printf '#!/usr/bin/env bash\nexit 1\n' > "$nested/test_helper.sh" # would fail the run if dispatched +printf 'import sys\nif __name__=="__main__": sys.exit(1)\n' > "$nested/test_helper.py" +invoke "$base" +if [[ "$CODE" == 0 ]] && [[ "$(jq -r .suites <<<"$OUT")" == 1 ]]; then + pass "nested test_* under tests/ -> not a suite (only the direct child counts)" +else + fail "nested exclusion: code=$CODE out=$OUT (expected suites=1, exit 0)" +fi +rm -rf "$base" + +# --- a suite exiting 125 is a FAILING SUITE, not a dispatcher fault --- +# An earlier draft used 125 as the dispatcher's private sentinel. Any +# reserved exit code is only a soft invariant: nothing stops a suite from +# returning it, and a real red suite then reports as a broken runner and +# stops the run. The fault now travels by side channel, so EVERY status a +# suite returns is its own verdict. This pins that. +base="$(make_base)"; add_suite "$base" alpha 0; add_suite "$base" sentinel125 125 +invoke "$base" +if [[ "$CODE" == 1 ]] \ + && [[ "$(jq -r .suites <<<"$OUT")" == 2 ]] \ + && [[ "$(jq -r .failed <<<"$OUT")" == 1 ]] \ + && jq -e '.failures | any(test("test_sentinel125.sh$"))' <<<"$OUT" >/dev/null; then + pass "suite exiting 125 -> counted as a failure, not a setup error" +else + fail "suite exit 125: code=$CODE out=$OUT" +fi +rm -rf "$base" + +# --- a missing interpreter is a SETUP error (2), not a test failure (1) --- +# Dispatching to an absent python3 gives the suite exit 127. Counting that +# as a failing suite reports red tests for a runner that never ran them, +# sending whoever reads CI to debug a test that never executed. +base="$(make_base)"; add_py_suite "$base" pyalpha 0 +errf="$(mktemp)" +# Override PY_BIN rather than emptying PATH — the harness needs bash, find, +# and mktemp on PATH to run at all, so nuking it tests the shebang, not the +# dispatcher. +OUT="$(PY_BIN=definitely-not-a-real-python3 "$RUNNER" "$base" 2>"$errf")"; CODE=$? +ERR="$(cat "$errf")"; rm -f "$errf" +if [[ "$CODE" == 2 ]] \ + && [[ "$(jq -r .suites <<<"$OUT")" == 0 ]] \ + && [[ "$(jq -r '.error' <<<"$OUT")" == *"interpreter missing"* ]]; then + pass "missing interpreter -> exit 2 (setup error), not exit 1" +else + fail "missing interpreter: code=$CODE out=$OUT err=$ERR" +fi +rm -rf "$base" + echo "" echo "run-tests.sh: ${PASS_COUNT} passed, ${FAIL_COUNT} failed" [[ $FAIL_COUNT -eq 0 ]] || exit 1 diff --git a/skills/release/SKILL.md b/skills/release/SKILL.md index 48bb0c75..2135248f 100644 --- a/skills/release/SKILL.md +++ b/skills/release/SKILL.md @@ -124,10 +124,17 @@ skills/release/dismiss-stale-reviews.sh Run it once Step 5's poll shows every bot's latest verdict clean. It emits a JSON summary of what it dismissed and what it left active, exits non-zero on API failure, and is idempotent on re-run. Which reviews it dismisses and which it leaves is the script's decision contract — see `skills/release/dismiss-stale-reviews.sh` header, not restated here (`rules/script-as-black-box.md`). -Before merging, capture the registry baseline so the post-merge check has something to compare against: +Before merging, capture the registry baseline so the post-merge check has something to compare against. Exit 0 emits `{"version": ""}`; exit 2 is a tool-state error, an unparseable registry output, or a version outside that shape. The script never emits an empty baseline, so a failure here stops the release. Accepted-shape rationale lives in the script header, not restated here (`rules/script-as-black-box.md`): + +Capture first, parse second — never one pipeline (the failure mechanism is in `skills/release/capture-registry-baseline.sh`'s header): ```bash -PRE=$(tessl plugin info / | grep "Latest Version" | awk '{print $NF}') +baseline_json=$(skills/release/capture-registry-baseline.sh ) \ + || { echo "Baseline capture failed — see stderr" >&2; exit 1; } +PRE=$(jq -r '.version' <<<"$baseline_json") \ + || { echo "Baseline JSON parse failed — see stderr" >&2; exit 1; } +[[ -n "$PRE" && "$PRE" != "null" ]] \ + || { echo "Baseline capture returned no version" >&2; exit 1; } ``` Pick the right cleanup path based on where you ran the skill from. @@ -173,17 +180,23 @@ Order in (B) is mandatory: `git branch -d` refuses to delete a branch that is ch After merge — per `rules/ci-safety.md`'s Always Watch CI duty extended through release: - Verify the merge landed on main (`git pull --ff-only` succeeds; `git log -1 --oneline` shows the merge commit) -- Watch the publish workflow to a terminal state. Bind to the merge commit SHA + the `push` event, never to "latest on main". The publish workflow may take several seconds to be enqueued after merge, so the run-id lookup polls until the run is listed (2s interval, 30s budget): +- Watch the publish workflow to a terminal state. Bind to the merge commit SHA + the `push` event, never to "latest on main". The resolver polls until the run is listed, absorbing the post-merge enqueue latency; its interval and budget are the script's constants (`skills/release/resolve-publish-run.sh` — top-of-file docstring), not restated here: + + Capture, then parse — never one pipeline (the failure mechanism is in `skills/release/resolve-publish-run.sh`'s header): ```bash merge_sha=$(gh pr view --json mergeCommit --jq '.mergeCommit.oid') - run_id=$(skills/release/resolve-publish-run.sh "$merge_sha" "" | jq -r '.database_id') + run_json=$(skills/release/resolve-publish-run.sh "$merge_sha" "") \ + || { echo "Publish run not resolved — see stderr" >&2; exit 1; } + run_id=$(jq -r '.database_id' <<<"$run_json") \ + || { echo "Resolver output is not valid JSON — see stderr" >&2; exit 1; } + [[ -n "$run_id" && "$run_id" != "null" ]] || { echo "Resolver returned no run id" >&2; exit 1; } gh run watch "$run_id" ``` No `--exit-status` on the watch: the publish-landed conjunction below reads the run conclusion explicitly, so letting `--exit-status` propagate a non-zero exit would short-circuit `set -e` wrappers before the conjunction runs. - `gh pr view` returns the specific merge commit for this PR, unaffected by parallel merges. The resolver filters on `headSha == $merge_sha` AND `event == push` so manual `workflow_dispatch` runs sharing the SHA are excluded; it also retries on enqueue latency so the immediate post-merge `gh run list` doesn't race the publish workflow's enqueue and surface as "no run found". Output is `{"database_id": N}` per `rules/script-delegation.md` — extract with `jq -r '.database_id'`. The watch is a timing precondition for the conjunction below, not the gate + `gh pr view` returns the specific merge commit for this PR, unaffected by parallel merges. The resolver's filter and retry logic live in its top-of-file docstring (`skills/release/resolve-publish-run.sh`), not restated here (`rules/script-as-black-box.md`). Output is `{"database_id": N}` per `rules/script-delegation.md` — extract with `jq -r '.database_id'`. The watch is a timing precondition for the conjunction below, not the gate - Confirm the publish landed via the conjunction check — conjuncts 1 and 2 (resolved run's `conclusion == success` AND registry's `Latest Version > PRE`). Capture the emitted `current` version for the moderation check that follows: ```bash diff --git a/skills/release/capture-registry-baseline.sh b/skills/release/capture-registry-baseline.sh new file mode 100755 index 00000000..73305917 --- /dev/null +++ b/skills/release/capture-registry-baseline.sh @@ -0,0 +1,135 @@ +#!/usr/bin/env bash +# Capture a tile's current registry version as the pre-merge baseline for +# the release contract's conjunct 2 (registry advanced past the baseline — +# see rules/ci-safety.md "Always Watch CI"). +# +# Why this is a script and not a pasted pipeline: skills/release/SKILL.md +# previously had the operator paste +# PRE=$(tessl plugin info /

| grep "Latest Version" | awk '{print $NF}') +# which shares its parse with verify-publish-landed.sh but none of its +# hardening. Two failure modes it could not see: +# 1. A tessl warning on stderr merged into the parsed text and misread +# the version. +# 2. A parse miss (the "Latest Version" line absent, e.g. the registry +# changed its output shape) silently yielded an empty PRE, which then +# went on to verify-publish-landed.sh AS the baseline — every version +# compares as "advanced past empty", so conjunct 2 passes vacuously +# and an unpublished release reports as confirmed. +# Same parse in two places, one unguarded, feeding the gate that is +# supposed to catch exactly this (rules/script-delegation.md — deterministic +# work belongs in a script, not a code block for the agent to reproduce). +# +# Usage: capture-registry-baseline.sh +# Out: one JSON object on stdout: {"version": ""} +# Numeric only — deliberately NARROWER than semver. The sole +# consumer is verify-publish-landed.sh's version_gt(), which orders +# baselines with bash arithmetic; a prerelease/build value arrives at +# (( )) as `3-rc.1+build.5`, and the arithmetic error makes the +# comparison return "not greater" rather than raising — so a landed +# publish reports as failed against an innocent registry. Widening +# this shape requires teaching version_gt() semver precedence first: +# the accepted shape and the comparator move together or not at all. +# Exit: 0 on a successful parse; 2 on tool-state error (tessl unreachable +# or absent, unparseable output, or a version outside the shape +# above). There is no exit 1: an absent +# baseline is never a valid input to the conjunction, so this script +# fails loud rather than emitting a value the caller might use. + +set -euo pipefail + +json_str() { + local s="$1" + s="${s//\\/\\\\}"; s="${s//\"/\\\"}" + printf '"%s"' "$s" +} + +# Script-global, NOT a main() local: the EXIT trap fires after main() +# returns on the happy path, and a local would be out of scope by then — +# `set -u` turns the cleanup into an unbound-variable failure that reddens +# a successful run. (verify-publish-landed.sh survives the same pattern +# only because every one of its paths exits from inside main().) +ERR_FILE="" +# The EXIT trap's final status leaks into the script's exit status, so +# cleanup must never speak for the script's outcome. Two distinct ways it +# used to: +# 1. A bare `[[ -n "$ERR_FILE" ]]` guard returning 1 (path never set, e.g. +# the usage-error path) rewrote a deliberate `exit 2` into `exit 1`. +# `return 0` fixes that one. +# 2. `return 0` alone does NOT fix the other: under `set -e` a failing +# `rm` aborts the handler before `return 0` ever runs, and the trap's +# status still rewrites the verdict. Reproduced with an unwritable +# TMPDIR — `exit 2` became `exit 1` and the handler never reached its +# last line. An `if` condition suspends `set -e`, so the rm failure is +# reported rather than escaping. +cleanup() { + if [[ -n "$ERR_FILE" ]]; then + if ! rm -f "$ERR_FILE"; then + echo "capture-registry-baseline.sh: warning: could not remove temp file ${ERR_FILE} — remove it by hand" >&2 + fi + fi + return 0 +} +trap cleanup EXIT + +main() { + if [[ $# -ne 2 ]]; then + echo "usage: capture-registry-baseline.sh " >&2 + exit 2 + fi + local workspace="$1" tile="$2" + + command -v tessl >/dev/null \ + || { echo "error: tessl CLI not found on PATH — install it ('npm i -g @tessl/cli') or add it to PATH, then re-run" >&2; exit 2; } + + ERR_FILE=$(mktemp) \ + || { echo "error: mktemp failed — cannot run capture-registry-baseline.sh without a writable TMPDIR" >&2; exit 2; } + + # Capture stdout and stderr separately, then parse stdout on its own. + # A mixed `2>&1` capture would let a tessl warning line reach the parse + # and be misread as the version. + local tessl_output + tessl_output=$(tessl plugin info "${workspace}/${tile}" 2>"$ERR_FILE") \ + || { local err; err=$(cat "$ERR_FILE"); echo "error: 'tessl plugin info ${workspace}/${tile}' failed: ${err} — verify (1) the workspace/plugin slug is correct, (2) you have network access to the registry, (3) 'tessl status' shows you are logged in, then re-run 'tessl plugin info ${workspace}/${tile}' directly to inspect the failure" >&2; exit 2; } + + # Parse in its own step so a miss is distinguishable from a tool failure. + # `grep` exits 1 on no-match, which `set -e` + `pipefail` would turn into + # a bare exit before the actionable diagnostic below fires; an explicit + # `if` around the pipeline keeps the failure observable without + # suppressing it. + local version="" + if ! version=$(printf '%s\n' "$tessl_output" | grep "Latest Version" | awk '{print $NF}'); then + version="" + fi + if [[ -z "$version" ]]; then + echo "error: could not parse 'Latest Version' from 'tessl plugin info ${workspace}/${tile}' output — the registry output shape may have changed; inspect it directly and update this parse (output was: ${tessl_output})" >&2 + exit 2 + fi + + # Validate the shape this script's contract promises. Taking the last + # field of the "Latest Version" line on faith accepts whatever the + # registry prints there — `Latest Version unavailable` would emit + # {"version":"unavailable"} and hand verify-publish-landed.sh a baseline + # its comparator cannot order, converting a registry-side outage into a + # bogus conjunct-2 verdict. + # + # Numeric major.minor.patch ONLY — deliberately narrower than semver. + # The sole consumer of this baseline is verify-publish-landed.sh's + # version_gt(), which splits on '.' and feeds the fields to bash + # arithmetic. A prerelease/build value reaches `(( ))` as `3-rc.1+build.5`, + # and the resulting arithmetic error makes the comparison return "not + # greater" rather than raising: a landed publish is then reported as + # "Latest Version is not greater than baseline — investigate the registry + # state", sending the operator after a registry that did nothing wrong. + # This script must not accept a baseline shape its consumer cannot order. + # Widening here requires teaching version_gt() full semver precedence + # first (see verify-publish-landed.sh) — the accepted shape and the + # comparator move together or not at all. + if [[ ! "$version" =~ ^[0-9]+\.[0-9]+\.[0-9]+$ ]]; then + echo "error: parsed 'Latest Version' token '${version}' from 'tessl plugin info ${workspace}/${tile}' is not a numeric major.minor.patch version — the registry may be reporting an outage, or is publishing a prerelease/build version that verify-publish-landed.sh's comparator cannot order; inspect it directly before retrying the release (output was: ${tessl_output})" >&2 + exit 2 + fi + + printf '{"version":%s}\n' "$(json_str "$version")" +} + +[[ "${BASH_SOURCE[0]}" == "${0}" ]] && main "$@" diff --git a/skills/release/tests/test_capture_registry_baseline.sh b/skills/release/tests/test_capture_registry_baseline.sh new file mode 100755 index 00000000..36aa982b --- /dev/null +++ b/skills/release/tests/test_capture_registry_baseline.sh @@ -0,0 +1,199 @@ +#!/usr/bin/env bash +# Outcome-based tests for capture-registry-baseline.sh. +# +# The contract under test is the one the pasted pipeline in SKILL.md could +# not hold: a parse miss or a tool failure must exit 2 with a diagnostic, +# never emit an empty baseline. An empty baseline is the dangerous output — +# it flows into verify-publish-landed.sh's conjunct 2, where every version +# compares as "advanced past empty", so an unpublished release confirms. +# +# Approach: run the script as a subprocess with a stub `tessl` executable +# placed first on PATH, so the real `set -euo pipefail`, the EXIT trap, and +# the exit codes are all exercised end-to-end — a sourced-function harness +# would approximate them and miss exactly the trap/exit-status defects this +# script hit in development. The stub selects its fixture via MOCK_MODE. +# jq is not used by the script, so stdout is asserted literally. +# +# Run: bash skills/release/tests/test_capture_registry_baseline.sh +# Exit 0 on all-pass; non-zero with a per-test diagnostic on failure. + +set -uo pipefail + +SCRIPT="$(cd "$(dirname "$0")/.." && pwd)/capture-registry-baseline.sh" +[[ -f "$SCRIPT" && -r "$SCRIPT" ]] || { echo "fatal: capture-registry-baseline.sh not readable at $SCRIPT" >&2; exit 2; } + +FAIL_COUNT=0 +PASS_COUNT=0 +pass() { PASS_COUNT=$((PASS_COUNT + 1)); echo " pass: $1"; } +fail() { FAIL_COUNT=$((FAIL_COUNT + 1)); echo " FAIL: $1" >&2; } + +# Invoke the script as a subprocess with a stubbed `tessl` on PATH, so the +# real `set -euo pipefail` and exit codes are exercised end-to-end rather +# than the sourced-function approximation of them. +# `set -e` is deliberately off here (a failure-counting harness must not die +# on its first red assertion), so an mktemp failure would otherwise leave +# STUBDIR empty, put "" on PATH, and produce a cascade of confusing +# assertion failures that look like the script's fault. +STUBDIR="$(mktemp -d)" || { echo "fatal: mktemp -d failed — cannot stage the tessl stub; check TMPDIR is writable" >&2; exit 2; } +[[ -n "$STUBDIR" && -d "$STUBDIR" ]] || { echo "fatal: mktemp -d returned no usable directory (got '${STUBDIR}')" >&2; exit 2; } +# Named handler ending `return 0`, not a bare `trap 'rm -rf ...'`: the EXIT +# trap's final command status becomes the process's exit status, so a failed +# cleanup (permissions, transient FS error) would turn an all-green run +# non-zero and flake CI. `set -e` is off here, so the rm cannot abort the +# handler — but its status still speaks for the run unless `return 0` does. +cleanup_stubdir() { + if [[ -n "${STUBDIR:-}" ]]; then + if ! rm -rf "$STUBDIR"; then + echo "warning: could not remove stub dir ${STUBDIR} — remove it by hand" >&2 + fi + fi + return 0 +} +trap cleanup_stubdir EXIT +export PATH="$STUBDIR:$PATH" + +# MOCK_MODE selects the fixture the stub emits. +cat > "$STUBDIR/tessl" <<'STUB' +#!/usr/bin/env bash +case "${MOCK_MODE:-}" in + ok) + echo "Plugin jbaruch/coding-policy" + echo " Latest Version 0.3.91" + ;; + warning_on_stderr) + echo "warn: registry cache stale, refetching" >&2 + echo " Latest Version 0.3.91" + ;; + no_version_line) + echo "Plugin jbaruch/coding-policy" + echo " Description nothing useful here" + ;; + version_not_semver) + # The registry reporting an outage on the line we parse. Taking the + # last field on faith yielded {"version":"unavailable"} — a baseline + # verify-publish-landed.sh's version comparison cannot order. + echo "Plugin jbaruch/coding-policy" + echo " Latest Version unavailable" + ;; + version_is_prose) + echo " Latest Version see registry status page" + ;; + tessl_fails) + echo "error: not authenticated" >&2 + exit 1 + ;; + *) echo "stub tessl: unknown MOCK_MODE='${MOCK_MODE:-}'" >&2; exit 99 ;; +esac +STUB +chmod +x "$STUBDIR/tessl" + +invoke() { OUT="$(bash "$SCRIPT" jbaruch coding-policy 2>"$STUBDIR/err")"; CODE=$?; ERR="$(cat "$STUBDIR/err")"; } + +echo "capture-registry-baseline.sh tests" + +# --- happy path: version parsed, emitted as JSON --- +MOCK_MODE=ok invoke +if [[ "$CODE" == 0 ]] && [[ "$OUT" == '{"version":"0.3.91"}' ]]; then + pass "parses Latest Version -> {\"version\":\"0.3.91\"}" +else + fail "happy path: code=$CODE out=$OUT err=$ERR" +fi + +# --- a tessl warning on stderr must not poison the parse --- +# This is the failure the pasted `tessl plugin info | grep | awk` could not +# see: with stdout and stderr merged, the warning line reaches the parse. +MOCK_MODE=warning_on_stderr invoke +if [[ "$CODE" == 0 ]] && [[ "$OUT" == '{"version":"0.3.91"}' ]]; then + pass "stderr warning does not poison the parsed version" +else + fail "stderr warning: code=$CODE out=$OUT err=$ERR" +fi + +# --- parse miss: exit 2, diagnostic, and NO empty baseline on stdout --- +# The pasted pipeline yielded an empty PRE here, which then satisfied +# conjunct 2 vacuously. +MOCK_MODE=no_version_line invoke +if [[ "$CODE" == 2 ]] && [[ -z "$OUT" ]] && [[ "$ERR" == *"could not parse 'Latest Version'"* ]]; then + pass "parse miss -> exit 2, empty stdout, actionable diagnostic" +else + fail "parse miss: code=$CODE out=$OUT err=$ERR" +fi + +# --- a non-version token on the Latest Version line is exit 2, not a value --- +# The contract promises numeric major.minor.patch. Emitting whatever the +# registry printed hands verify-publish-landed.sh a baseline its comparator +# cannot order — a registry outage silently becomes a bogus conjunct-2 +# verdict. +MOCK_MODE=version_not_semver invoke +if [[ "$CODE" == 2 ]] && [[ -z "$OUT" ]] && [[ "$ERR" == *"not a numeric major.minor.patch"* ]]; then + pass "non-version token ('unavailable') -> exit 2, empty stdout" +else + fail "non-semver: code=$CODE out=$OUT err=$ERR" +fi + +# --- multi-word prose on the version line is also rejected --- +# awk '{print $NF}' takes the LAST field, so prose yields a plausible-looking +# single token ("page") that only a shape check catches. +MOCK_MODE=version_is_prose invoke +if [[ "$CODE" == 2 ]] && [[ -z "$OUT" ]] && [[ "$ERR" == *"not a numeric major.minor.patch"* ]]; then + pass "prose version line -> exit 2, empty stdout" +else + fail "prose version: code=$CODE out=$OUT err=$ERR" +fi + +# --- a prerelease/build version is REJECTED, not accepted --- +# The accepted shape is bounded by the consumer, not by semver. This +# baseline's only reader is verify-publish-landed.sh's version_gt(), which +# splits on '.' and feeds the fields to bash arithmetic: `1.2.3-rc.1+build.5` +# arrives at `(( ))` as `3-rc.1+build.5`, and the arithmetic error makes the +# comparison return "not greater" instead of raising — so a landed publish +# reports as "Latest Version is not greater than baseline — investigate the +# registry state". Accepting a shape the consumer cannot order buys a false +# release verdict, so this script rejects it at the boundary. +cat > "$STUBDIR/tessl" <<'STUB2' +#!/usr/bin/env bash +echo " Latest Version 1.2.3-rc.1+build.5" +STUB2 +chmod +x "$STUBDIR/tessl" +invoke +if [[ "$CODE" == 2 ]] && [[ -z "$OUT" ]] && [[ "$ERR" == *"not a numeric major.minor.patch"* ]]; then + pass "prerelease+build version -> exit 2 (comparator cannot order it)" +else + fail "prerelease rejection: code=$CODE out=$OUT err=$ERR" +fi +# restore the MOCK_MODE stub for the remaining cases +cat > "$STUBDIR/tessl" <<'STUB' +#!/usr/bin/env bash +case "${MOCK_MODE:-}" in + ok) + echo "Plugin jbaruch/coding-policy" + echo " Latest Version 0.3.91" + ;; + tessl_fails) + echo "error: not authenticated" >&2 + exit 1 + ;; + *) echo "stub tessl: unknown MOCK_MODE='${MOCK_MODE:-}'" >&2; exit 99 ;; +esac +STUB +chmod +x "$STUBDIR/tessl" + +# --- tessl itself failing is exit 2 with the tool's stderr surfaced --- +MOCK_MODE=tessl_fails invoke +if [[ "$CODE" == 2 ]] && [[ -z "$OUT" ]] && [[ "$ERR" == *"not authenticated"* ]]; then + pass "tessl failure -> exit 2, tool stderr surfaced" +else + fail "tessl failure: code=$CODE out=$OUT err=$ERR" +fi + +# --- wrong arity is a usage error, not a silent default --- +OUT="$(bash "$SCRIPT" jbaruch 2>"$STUBDIR/err")"; CODE=$?; ERR="$(cat "$STUBDIR/err")" +if [[ "$CODE" == 2 ]] && [[ "$ERR" == *"usage:"* ]]; then + pass "wrong arity -> exit 2 with usage" +else + fail "arity: code=$CODE out=$OUT err=$ERR" +fi + +echo "" +echo "capture-registry-baseline.sh: ${PASS_COUNT} passed, ${FAIL_COUNT} failed" +[[ $FAIL_COUNT -eq 0 ]] || exit 1 diff --git a/skills/release/verify-publish-landed.sh b/skills/release/verify-publish-landed.sh index 9c4260de..e1106dec 100755 --- a/skills/release/verify-publish-landed.sh +++ b/skills/release/verify-publish-landed.sh @@ -104,7 +104,7 @@ main() { exit 2 fi if [[ -z "$pre" ]]; then - echo "error: is empty — capture with 'tessl plugin info ${workspace}/${tile} | grep \"Latest Version\" | awk \"{print \\\$NF}\"' before merge" >&2 + echo "error: is empty — capture it before merge with 'skills/release/capture-registry-baseline.sh ${workspace} ${tile}' and read .version (never the raw 'tessl plugin info | grep | awk' pipeline, which this repo extracted into that guarded script)" >&2 exit 2 fi