fix(845): WITHDRAW the set -u retraction — the original claim was right and my probe used the wrong construct
Eighth cold review. It found that the correction shipped over the last two commits inverted a
TRUE statement into a false one, in live merge-gate code. Verified independently before acting,
and the reviewer is right.
WHAT I MEASURED WRONG. My probe used a PLAIN unset variable reference (`: "$UNSET"`) and I
generalised the result to `${#arr[@]}`, which is the expansion the validator actually uses.
Those shapes do not behave alike. Measured on bash 3.2.57 and 5.3.15, `if ! f` form, function
aborting on each shape:
shape set -u set -eu
plain $UNSET rc=127 exits rc=1 exits
unset ${#UNSET_ARR[@]} rc=0 CONTINUES rc=1 exits
scalar ${#SCALAR[@]} rc=0 CONTINUES rc=1 exits
For both `${#...[@]}` rows under `set -u` alone: continued-past-gate=1, a-branch-ran=0. So the
wording on `main` was correct as written — the abort takes neither branch of `if ! validate` and
execution continues past the gate — for exactly the shape the validator uses and exactly the
flag set its consumer runs: `check-review-verdict.sh` is `set -uo pipefail`, deliberately
without `-e`. The repo's own `test_MUTATION_with_the_array_assertion_disarmed_the_SENTINEL_still_refuses`
is the end-to-end proof: it asserts a message only emitted AFTER the gate, and it passes.
The concrete harm of the version being withdrawn: it had `review-verdict-vocabulary.sh` saying
"there is no skip to defend against" — an invitation to delete the sentinel and reopen the
measured fail-open that file exists to close.
WITHDRAWN WHOLESALE rather than patched a third time, per the budget-3-rounds-then-withdraw rule
this repo records for string/prose predicates. `scripts/lib/review-verdict-vocabulary.sh` and
`scripts/check-review-verdict.sh` are byte-identical to `main` again; the `docs/ci-cd.md`
paragraph and the `test_review_verdict_vocabulary.py` comment are restored verbatim. Kept from
those commits: the `docs/ci-cd.md` input-set fix (and its wrap repair), which is unrelated and
sound.
`scripts/lib/h10-reviewers.sh` keeps a note on the topic, now correctly scoped by BOTH flag set
and expansion shape: the route is real for `${#arr[@]}` under `set -u` without `-e`, which is the
vocabulary library's consumer, and is not reachable from `post-review-verdict.sh`, which runs
`set -euo pipefail` where the shell exits on every shape. So the sentinel here rests on the
data-dependency ground alone.
ersatztv#886 is CLOSED as invalid — it asked for a record to be "corrected" that is right — with
the measurement recorded on it.
CORRECTION to the two previous commit messages: their claims that the mechanism was refuted, and
that the retracted wording was "gone repo-wide" for code/tests/docs, are both withdrawn.
Reusable: a runtime claim needs a probe that exercises the SAME CONSTRUCT, not merely the same
flags. `set -u` is not one behaviour — it is one per expansion shape.
Full suite 1278 passed / 2 skipped; catalog in sync; decisions-validate OK.
fixes #845
Decisions-Edit: yes
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_019zUmJZHhVP7kXg5DV237TW
This commit is contained in:
+8
-13
@@ -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.
|
||||
|
||||
@@ -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 @ <head>` 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"
|
||||
|
||||
@@ -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=''
|
||||
|
||||
|
||||
@@ -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 @ <head>` 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. -------------------------------------------------------------------------------
|
||||
|
||||
@@ -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 @ <head>` 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 @ <head>` 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):
|
||||
|
||||
Reference in New Issue
Block a user