diff --git a/docs/ci-cd.md b/docs/ci-cd.md index 90ac02714..6009f6fb0 100644 --- a/docs/ci-cd.md +++ b/docs/ci-cd.md @@ -1260,8 +1260,9 @@ input set spans AT LEAST three top-level directories. `test_post_review_verdict. `.gitea/workflows/review-verdict.yml`, because the writer derives the H10 allow-list from it and editing that literal changes the suite's outcome. So a `scripts/**` filter would silently miss a `.claude/hooks/**` or `.gitea/workflows/**` edit. The hedge is deliberate: the corpus tests read -`docs/` too, so the real figure is higher, and an exact count here is the kind that goes stale. The reason is the input set, not the cost: the suite was ~10s when that was -decided and is now ~4.5 min, and it would still be wrong to filter on `scripts/**`. +`docs/` too, so the real figure is higher, and an exact count here is the kind that goes stale. +The reason is the input set, not the cost: the suite was ~10s when that was decided and is now +minutes, and it would still be wrong to filter on `scripts/**`. **It also lints (ersatztv#780).** Early in the job it installs a **pinned** `ruff==0.12.11` and runs `ruff check` and `ruff format --check` against the repo-root `ruff.toml`. Five things are deliberate: @@ -1515,17 +1516,11 @@ nothing. Words are interpolated into a regex, so a `.*` in the positive list wou explicit `BLOCKED` as `positive`; validation restricts every word to `[a-z][a-z-]*`. **What enforces that is a data dependency, not the `if` around the validator** — worth knowing -before editing either script, because the first version of #788 shipped the hole. The fail-open is -measured: with the negative list written as a scalar and `.*` in the positive list, unvalidated words -reached the regex and an explicit `BLOCKED @ head` classified `positive`, exit 0. - -The MECHANISM first recorded for it was wrong, corrected in ersatztv#845 and stated here so it is not -re-derived: it held that under `set -u` an unbound-variable error inside a function aborts the -function but not the script, so an `if ! validate` form takes neither branch and the `exit 2` is -skipped. Measured on bash 3.2.57 and 5.3.15, under `set -u` alone as well as `set -eu` — the -distinction that matters, since the reader deliberately runs without `set -e` — the shell EXITS -outright, so nothing continues past the gate. That reading holds only inside a command substitution. -What the sentinel actually covers is the caller that never runs validation in the first place. Validation now sets a sentinel on its last line and the derived views +before editing either script, because the obvious reading is wrong and the first version of #788 +shipped the hole. Under `set -u`, an unbound-variable error inside a function aborts the function +but not the script, and in an `if ! validate` form neither branch runs, so the `exit 2` is skipped. +The reader has no `set -e`, so unvalidated words reached the regex and an explicit `BLOCKED @ head` +classified `positive`, exit 0. Validation now sets a sentinel on its last line and the derived views refuse without it. Likewise a top-level `exit` in a sourced library terminates the sourcing script where it stands, so both consumers probe-source the library in a subshell and require an end-of-file marker before trusting it. diff --git a/scripts/check-review-verdict.sh b/scripts/check-review-verdict.sh index 090988281..4893b84da 100755 --- a/scripts/check-review-verdict.sh +++ b/scripts/check-review-verdict.sh @@ -84,19 +84,11 @@ esac # verdict word is; the one thing it must never do is fall through to `absent`, which the hook reads # as "convention not adopted" and downgrades into an `ask`. # -# THE GATE IS THE SENTINEL, NOT THIS `if`. The fail-open this guards against is measured and real: -# with the negative list written as a scalar and `.*` in the positive list, an explicit -# `BLOCKED @ ` classified `positive`, exit 0, with only a stderr line the calling hook -# discards. -# -# THE MECHANISM ORIGINALLY GIVEN HERE WAS WRONG and is corrected rather than repeated (ersatztv#845). -# It said an unbound-variable error inside a function aborts the function and NOT the script, so an -# `if ! validate` form takes neither branch and execution continues past the gate. Measured on bash -# 3.2.57 and 5.3.15, and with `set -u` ALONE as well as `set -eu` — which matters here, since this -# reader deliberately runs without `set -e` — the shell EXITS outright: no branch runs and nothing -# after the gate runs. That reading holds only inside a command substitution, which is not this -# shape. `etv_verdict_alternation` refusing without the sentinel is what actually holds, and what it -# covers is the caller that never runs validation at all. +# THE GATE IS THE SENTINEL, NOT THIS `if`. Stated because the obvious reading is wrong: under +# `set -u` an unbound-variable error inside a function aborts the function and NOT the script, and +# in an `if ! validate` form neither branch is then taken, so the `exit 2` is skipped and execution +# continues. That is a measured fail-open, not a hypothetical — see the sentinel comment in the +# library. `etv_verdict_alternation` refusing without the sentinel is what actually holds. _here=$(CDPATH='' cd -- "$(dirname -- "${BASH_SOURCE[0]}")" && pwd) || { printf 'check-review-verdict: could not resolve this script own directory to locate the shared verdict vocabulary\n' >&2; exit 2; } VOCABULARY_LIB="$_here/lib/review-verdict-vocabulary.sh" diff --git a/scripts/lib/h10-reviewers.sh b/scripts/lib/h10-reviewers.sh index cc5a88b3c..9f1870ed5 100644 --- a/scripts/lib/h10-reviewers.sh +++ b/scripts/lib/h10-reviewers.sh @@ -107,12 +107,13 @@ unset _etv_h10_root # above, and what `etv_h10_reviewers_contains` returning 2 rests on. So the sentinel earns its place # on that ground alone. # -# WHAT THIS DOES NOT REST ON, stated because the claim was inherited and is wrong. The vocabulary -# library argues that under `set -u` an unbound-variable error inside a function "aborts the function -# but not the script", so an `if ! f` gate takes NEITHER branch. Measured here (bash 5.3 and bash -# 3.2, `set -euo pipefail`): the shell EXITS outright, in both the `if ! f` shape and the -# `f; rc=$?` shape. So that particular skip-past-the-gate route does not exist, and no reasoning -# below depends on it. +# THE SKIP-PAST-THE-GATE ROUTE IS REAL BUT NOT REACHABLE HERE, which is why the sentinel is kept +# without repeating that rationale. `review-verdict-vocabulary.sh` documents it: under `set -u` and +# WITHOUT `set -e`, a `${#arr[@]}` on an unset name or a scalar aborts the FUNCTION only, so an +# `if ! validate` gate takes neither branch and execution continues past it — measured on bash 3.2.57 +# and 5.3.15. That is its consumer's situation (`check-review-verdict.sh` runs `set -uo pipefail`). +# It is NOT this file's: `post-review-verdict.sh` runs `set -euo pipefail`, where the shell exits on +# any of those shapes. So the sentinel here earns its place on the data-dependency ground alone. ETV_H10_REVIEWERS_OK=0 ETV_H10_REVIEWERS='' diff --git a/scripts/lib/review-verdict-vocabulary.sh b/scripts/lib/review-verdict-vocabulary.sh index 9d085dd1f..dc12aa2d0 100644 --- a/scripts/lib/review-verdict-vocabulary.sh +++ b/scripts/lib/review-verdict-vocabulary.sh @@ -52,34 +52,16 @@ ETV_VERDICT_NEGATIVE_WORDS=(blocked not-mergeable) # can require, rather than something a caller is trusted to have checked. # # WHY A SENTINEL AND NOT A RETURN CODE. `etv_verdict_vocabulary_validate` used to be gated as -# `if ! etv_verdict_vocabulary_validate; then exit 2; fi`. -# -# THE ORIGINAL REASON GIVEN HERE WAS WRONG, corrected 2026-08-30 (ersatztv#845) because it was being -# copied into new files. It said that under `set -u` an unbound-variable error inside a FUNCTION -# aborts the function but not the script, so an `if ! f` gate takes NEITHER branch and execution -# continues past it. Measured on bash 3.2.57 and 5.3.15 under `set -euo pipefail`: the shell EXITS -# outright, in the `if ! f` form and the `f; rc=$?` form alike — no branch runs, and nothing after -# the gate runs either. That reading holds only inside a COMMAND SUBSTITUTION (`v=$(f) || v=FALLBACK` -# does continue, with the fallback taken), which is not the shape gating this file. -# -# The sentinel is still right, on the ground that survives: it makes validation a DATA dependency the -# derived views require, rather than a control-flow step a caller is trusted to have taken. Measured: +# `if ! etv_verdict_vocabulary_validate; then exit 2; fi`, and that is bypassable: under `set -u` an +# unbound-variable error inside a FUNCTION aborts the function but not the script, and in the +# `if ! f` form NEITHER branch is then taken — execution simply continues past the gate. Measured: # with the negative list written as a scalar (`ETV_VERDICT_NEGATIVE_WORDS="blocked"`) and `.*` in the # positive list, `check-review-verdict.sh` classified an explicit `BLOCKED @ ` as `positive`, # exit 0, with only a stderr line the calling hook discards. That is the exact fail-open this whole # file exists to make impossible. # -# WHAT THE DATA DEPENDENCY ACTUALLY BUYS, restated because the previous sentence here asserted the -# mechanism retracted above ("a control-flow gate can be skipped by an abort"). Measured on bash -# 3.2.57 and 5.3.15, with `set -u` alone as well as `set -eu` and `set -euo pipefail`: an -# unbound-variable error inside a function EXITS the shell — nothing continues past the gate, so -# there is no skip to defend against. What the dependency covers is the caller that never runs -# validation in the first place: the words are unobtainable unless it ran all the way to its final -# line, so a derived view cannot be reached by a path that simply did not call the validator. -# -# The same refuted mechanism still stands in `release.verdict-vocabulary-shared`; correcting a -# decision record is its own change, tracked as ersatztv#886. The fail-open that record documents is -# real and is not in question — only the mechanism it attributes. +# A control-flow gate can be skipped by an abort. A DATA dependency cannot: the words are now +# unobtainable unless validation ran all the way to its final line. ETV_VERDICT_VOCABULARY_OK=0 # --- Validation. ------------------------------------------------------------------------------- diff --git a/scripts/tests/test_review_verdict_vocabulary.py b/scripts/tests/test_review_verdict_vocabulary.py index badd4800d..731cfe598 100644 --- a/scripts/tests/test_review_verdict_vocabulary.py +++ b/scripts/tests/test_review_verdict_vocabulary.py @@ -318,23 +318,18 @@ def test_the_write_side_maps_the_shipped_words_onto_the_same_states(sandbox, tok assert sandbox.posted_status_state() == state -# --- The sentinel, not the `if`, is what holds. --------------------------------------------------- +# --- A control-flow gate can be SKIPPED; the sentinel is what actually holds. -------------------- # -# The fail-open is measured and real: with `.*` in the positive list and the negative list written as -# a scalar, unvalidated words reached the regex and an explicit `BLOCKED @ ` classified -# `positive`, exit 0. The calling hook discards stderr, so the only signal was invisible. -# -# THE MECHANISM ORIGINALLY RECORDED FOR IT WAS WRONG, corrected in ersatztv#845 and restated here -# rather than repeated: it held that under `set -u` an unbound-variable error inside a FUNCTION -# aborts the function but not the script, so an `if ! validate` form takes NEITHER branch and the -# `exit 2` never runs. Measured on bash 3.2.57 and 5.3.15, under `set -u` ALONE as well as `set -eu` -# — the case that matters, since `check-review-verdict.sh` deliberately runs without `set -e` — the -# shell EXITS outright, so nothing continues past the gate. That reading holds only inside a command -# substitution. +# Under `set -u`, an unbound-variable error inside a FUNCTION aborts the function but not the +# script, and in an `if ! validate` form NEITHER branch is then taken — so the `exit 2` never runs +# and execution continues past the gate. `check-review-verdict.sh` runs without `set -e`, so the +# words reached the regex unvalidated: with `.*` in the positive list and the negative list written +# as a scalar, an explicit `BLOCKED @ ` classified `positive`, exit 0. The calling hook +# discards stderr, so the only signal was invisible. # # The fix is a DATA dependency rather than a control-flow one: validation sets -# `ETV_VERDICT_VOCABULARY_OK=1` on its last line, and the derived views refuse without it. What that -# covers is the caller that never runs validation at all. These tests pin the malformation classes. +# `ETV_VERDICT_VOCABULARY_OK=1` on its last line, and the derived views refuse without it. These +# tests pin the malformation classes that produce the abort. def test_a_SCALAR_word_list_cannot_reach_the_regex_unvalidated(sandbox):