fix(788): one declarative H10 verdict vocabulary, derived by both sides (#846)
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 6s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 25s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m34s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m17s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 5m50s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 4m22s
Build ErsatzTV Image / CI toolchain image resolves (push) Successful in 6s
Build ErsatzTV Image / Delimiter ban (release path) (push) Successful in 25s
Build ErsatzTV Image / Build & test (.NET) (push) Successful in 8m34s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (push) Successful in 6m17s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (push) Successful in 5m50s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (push) Skipped
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (push) Skipped
Build ErsatzTV Image / Build & push image (amd64) (push) Successful in 4m22s
The verdict words lived in two hand-written shell copies — the `case` arms of post-review-verdict.sh (write) and the POS_RE/NEG_RE regexes of check-review-verdict.sh (read) — held together by nothing but a comment that had already gone stale. scripts/lib/review-verdict-vocabulary.sh now declares them once and both sides derive; neither script enumerates a verdict word any more. Only the WORD SET moved. The grammar stays in check-review-verdict.sh, where every #629 false-open actually lived. No parity test: #774 shipped one and withdrew it after six rounds, because a regex over shell source is not a shell parser. The proof is behavioural and graded MUTATION — the harness restores the pre-#788 hardcoded POS_RE each run and requires it to redden. Enforcement is a DATA dependency, not a control-flow gate. Review round 1 found a real fail-open in the first commit: `${#arr[@]}` is nounset-safe only for a declared-empty array, and under `set -u` that error inside a function called as `if ! validate` skips BOTH branches — so on the reader (deliberately no `set -e`) an explicit BLOCKED @ head classified `positive`, exit 0. Validation now sets a sentinel on its last line and the derived views refuse without it. Six cold review rounds; rounds 2-6 found no fail-open across differential fuzzing (4788 / 2612 / 7560 payloads, zero divergences from origin/main's grammar), sentinel forgery, environment poisoning, declare -p evasion on bash 5.3 and 3.2, path/symlink resolution and probe TOCTOU. Every malformation fails closed: reader exit 2, writer exit 1 with nothing posted. Also corrected: CLAUDE.md and release.review-verdict-gate both enumerated the vocabulary without LGTM, a word the code has accepted since #629. fixes #788 Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>
This commit was merged in pull request #846.
This commit is contained in:
@@ -0,0 +1,231 @@
|
||||
# shellcheck shell=bash
|
||||
# review-verdict-vocabulary.sh — THE H10 verdict vocabulary, declared ONCE (ersatztv#788).
|
||||
#
|
||||
# SOURCED, NEVER EXECUTED. It defines data plus pure functions and takes no action of its own, so it
|
||||
# is deliberately not executable and carries no shebang.
|
||||
#
|
||||
# WHY THIS FILE EXISTS. The vocabulary used to live in two hand-written shell copies: the `case`
|
||||
# arms of `scripts/post-review-verdict.sh` (the WRITE side, which turns a word into a commit-status
|
||||
# state) and the `POS_RE`/`NEG_RE` regexes of `scripts/check-review-verdict.sh` (the READ side,
|
||||
# which classifies a `Review-verdict:` comment for the merge-consent hook). A word the write side
|
||||
# treats as positive but the read side does not sends the server-side status green while the hook
|
||||
# still denies — an unexplained deny on a gate whose whole job is to be explicable.
|
||||
#
|
||||
# WHY IT IS NOT A TEST. ersatztv#774 tried to hold the two copies together with a parity test that
|
||||
# extracted both vocabularies from their own shell source with regexes and compared them. Six cold
|
||||
# review rounds each found another shell construction that either escaped the extractor (`SHIP*)`, a
|
||||
# glob in the arm label; an unquoted `SHIP-IT) state=success`) or reddened it on a correct tree (a
|
||||
# `<word>) state=` inside a heredoc; a column-zero `esac` in a string truncating the scoped match).
|
||||
# Each fix was locally correct and the sequence converged on nothing, because a regex over shell
|
||||
# source is not a shell parser. It was withdrawn rather than patched a seventh time — see
|
||||
# `docs/decisions/records/testing/guard-derives-population-from-source.md`, which carries it as the
|
||||
# worked example of "a weak detector is itself the symptom-keyed mistake".
|
||||
#
|
||||
# So this file removes the duplication rather than detecting it. There is one list; both sides
|
||||
# DERIVE from it. Do not reintroduce a text-comparison test alongside — the lesson of those six
|
||||
# rounds is that the shape does not converge, and dedup by construction is what closes it.
|
||||
#
|
||||
# ADDING A WORD is a one-line edit to the list below, and it reaches both sides by construction.
|
||||
# `scripts/tests/test_review_verdict_vocabulary.py` proves exactly that, by adding a word to a
|
||||
# sandbox copy of THIS file and driving both real scripts.
|
||||
|
||||
# --- The vocabulary. Whitespace-separated, lower case. -------------------------------------------
|
||||
# Positive: the merge may proceed -> commit status `success`.
|
||||
# Negative: the merge stays blocked -> commit status `failure`.
|
||||
# A word in NEITHER list is `unknown` on the read side and a hard refusal on the write side. That is
|
||||
# deliberate and must stay: a token nobody recognises is surfaced for a human, never guessed at.
|
||||
#
|
||||
# DECLARED AS ARRAYS, not as whitespace-separated strings, and every expansion below is quoted
|
||||
# (`"${arr[@]}"`). A string list has to be split by an UNQUOTED expansion, and an unquoted expansion
|
||||
# also performs PATHNAME EXPANSION: with `.*` in the list the validator below reported on `.claude`,
|
||||
# a file in the repo root, rather than on the word actually written. That makes validation depend on
|
||||
# the working directory, and it fails OPEN in the case that matters — a `*` entry expands to the
|
||||
# filenames around it, and any of those matching `[a-z][a-z-]*` (`scripts`, `docs`) would validate
|
||||
# cleanly and enter the vocabulary as a real verdict word. Arrays remove the expansion rather than
|
||||
# guarding it.
|
||||
ETV_VERDICT_POSITIVE_WORDS=(mergeable approved lgtm)
|
||||
ETV_VERDICT_NEGATIVE_WORDS=(blocked not-mergeable)
|
||||
|
||||
# --- The validation sentinel. -------------------------------------------------------------------
|
||||
# Reset to 0 HERE, at load time, for two reasons. It defeats an inherited `ETV_VERDICT_VOCABULARY_OK=1`
|
||||
# from the environment, and — the reason it exists — it makes "validated" a fact the DERIVED VIEWS
|
||||
# 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`, 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.
|
||||
#
|
||||
# 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. -------------------------------------------------------------------------------
|
||||
# The read side interpolates these words into an EXTENDED REGULAR EXPRESSION. That makes the list an
|
||||
# injection surface, and the failure direction is the dangerous one: a stray `.*` in the positive
|
||||
# list would make POS_RE match every verdict-shaped line, so an explicit `BLOCKED` would classify as
|
||||
# `positive` and the merge gate would grant consent it was never given. Restricting words to
|
||||
# `[a-z][a-z-]*` means no character reaching the regex can be a metacharacter, which is a property of
|
||||
# the character class rather than of anyone remembering to escape.
|
||||
#
|
||||
# Returns 0 when the vocabulary is usable, 1 otherwise (message on stderr). It never exits: the two
|
||||
# consumers fail closed with DIFFERENT codes (`post` dies 1, `check` exits 2 so its callers treat it
|
||||
# as an unreadable input), so the exit semantics belong to them, not here.
|
||||
# True when $1 names a set variable carrying the ARRAY attribute. Checked before any `${#name[@]}`,
|
||||
# because that expansion is nounset-safe ONLY for a declared-empty array: on an UNSET name or on a
|
||||
# SCALAR it is a fatal unbound-variable error, which is precisely the abort that used to skip
|
||||
# validation entirely. `declare -p` answers without expanding anything.
|
||||
etv_verdict__is_array() {
|
||||
local declaration flags
|
||||
declaration=$(declare -p "$1" 2>/dev/null) || return 1
|
||||
flags=${declaration#declare -}
|
||||
flags=${flags%% *}
|
||||
case "$flags" in
|
||||
*a*) return 0 ;;
|
||||
*) return 1 ;;
|
||||
esac
|
||||
}
|
||||
|
||||
etv_verdict_vocabulary_validate() {
|
||||
local word seen=' ' name
|
||||
|
||||
ETV_VERDICT_VOCABULARY_OK=0
|
||||
|
||||
for name in ETV_VERDICT_POSITIVE_WORDS ETV_VERDICT_NEGATIVE_WORDS; do
|
||||
if ! etv_verdict__is_array "$name"; then
|
||||
printf 'review-verdict vocabulary: %s is not an array — declare it as %s=(word word), not as a string\n' "$name" "$name" >&2
|
||||
return 1
|
||||
fi
|
||||
done
|
||||
|
||||
if [ "${#ETV_VERDICT_POSITIVE_WORDS[@]}" -eq 0 ] || [ "${#ETV_VERDICT_NEGATIVE_WORDS[@]}" -eq 0 ]; then
|
||||
printf 'review-verdict vocabulary: the positive and negative lists must both be non-empty\n' >&2
|
||||
return 1
|
||||
fi
|
||||
|
||||
for word in "${ETV_VERDICT_POSITIVE_WORDS[@]}" "${ETV_VERDICT_NEGATIVE_WORDS[@]}"; do
|
||||
# Rejects upper case, digits, whitespace and every regex metacharacter, plus a leading or
|
||||
# trailing hyphen (which would build an alternation branch that reads oddly and matches nothing
|
||||
# useful). `-` is last in the bracket expression, where it is literal.
|
||||
case "$word" in
|
||||
'' | *[!a-z-]* | -* | *-)
|
||||
printf 'review-verdict vocabulary: %s is not a usable verdict word (allowed: [a-z] and internal -)\n' "$word" >&2
|
||||
return 1
|
||||
;;
|
||||
esac
|
||||
# A word in BOTH lists would classify as whichever side is consulted first — an ambiguity that
|
||||
# would silently pick a side. Reject it here instead.
|
||||
case "$seen" in
|
||||
*" $word "*)
|
||||
printf 'review-verdict vocabulary: %s appears twice; a word must be positive or negative, not both\n' "$word" >&2
|
||||
return 1
|
||||
;;
|
||||
esac
|
||||
seen="$seen$word "
|
||||
done
|
||||
|
||||
# The ONLY assignment to 1 in this file, and it is the last statement of the successful path. Any
|
||||
# earlier `return 1`, and any abort part-way through, leaves the sentinel at 0.
|
||||
ETV_VERDICT_VOCABULARY_OK=1
|
||||
return 0
|
||||
}
|
||||
|
||||
# --- Derived view 1: classify one token (the WRITE side). ---------------------------------------
|
||||
# Prints `positive` or `negative` and returns 0; returns 1 for a word in neither list, printing
|
||||
# nothing. Case-insensitive, so the caller may pass the operator's raw argument.
|
||||
etv_verdict_class() {
|
||||
local token word
|
||||
if [ "${ETV_VERDICT_VOCABULARY_OK:-0}" != 1 ]; then
|
||||
printf 'review-verdict vocabulary: not validated — refusing to hand out verdict words\n' >&2
|
||||
return 1
|
||||
fi
|
||||
|
||||
token=$(printf '%s' "${1-}" | tr '[:upper:]' '[:lower:]')
|
||||
|
||||
for word in ${ETV_VERDICT_POSITIVE_WORDS[@]+"${ETV_VERDICT_POSITIVE_WORDS[@]}"}; do
|
||||
if [ "$word" = "$token" ]; then
|
||||
printf 'positive\n'
|
||||
return 0
|
||||
fi
|
||||
done
|
||||
for word in ${ETV_VERDICT_NEGATIVE_WORDS[@]+"${ETV_VERDICT_NEGATIVE_WORDS[@]}"}; do
|
||||
if [ "$word" = "$token" ]; then
|
||||
printf 'negative\n'
|
||||
return 0
|
||||
fi
|
||||
done
|
||||
|
||||
return 1
|
||||
}
|
||||
|
||||
# --- Derived view 2: an ERE alternation body (the READ side). -----------------------------------
|
||||
# `etv_verdict_alternation positive` -> `mergeable|approved|lgtm`. The caller wraps it in its own
|
||||
# anchors and boundary, so this deliberately emits no parentheses: the grammar around the token
|
||||
# belongs to the classifier, only the WORD SET belongs here.
|
||||
etv_verdict_alternation() {
|
||||
local alternation='' word
|
||||
local words=()
|
||||
if [ "${ETV_VERDICT_VOCABULARY_OK:-0}" != 1 ]; then
|
||||
printf 'review-verdict vocabulary: not validated — refusing to hand out verdict words\n' >&2
|
||||
return 1
|
||||
fi
|
||||
|
||||
case "${1-}" in
|
||||
positive) words=(${ETV_VERDICT_POSITIVE_WORDS[@]+"${ETV_VERDICT_POSITIVE_WORDS[@]}"}) ;;
|
||||
negative) words=(${ETV_VERDICT_NEGATIVE_WORDS[@]+"${ETV_VERDICT_NEGATIVE_WORDS[@]}"}) ;;
|
||||
*)
|
||||
printf 'review-verdict vocabulary: etv_verdict_alternation needs positive|negative, got %s\n' "${1-}" >&2
|
||||
return 1
|
||||
;;
|
||||
esac
|
||||
|
||||
for word in ${words[@]+"${words[@]}"}; do
|
||||
if [ -z "$alternation" ]; then alternation="$word"; else alternation="$alternation|$word"; fi
|
||||
done
|
||||
|
||||
[ -n "$alternation" ] || return 1
|
||||
printf '%s\n' "$alternation"
|
||||
}
|
||||
|
||||
# --- Derived view 3: the operator-facing list. --------------------------------------------------
|
||||
# `MERGEABLE, APPROVED, LGTM, BLOCKED or NOT-MERGEABLE`. Derived rather than written out, because
|
||||
# the "unknown verdict" error message naming a stale set is how an operator learns the wrong
|
||||
# vocabulary — the same drift this file exists to remove, one layer out.
|
||||
etv_verdict_words_display() {
|
||||
local word index=0 total out=''
|
||||
if [ "${ETV_VERDICT_VOCABULARY_OK:-0}" != 1 ]; then
|
||||
printf 'review-verdict vocabulary: not validated — refusing to hand out verdict words\n' >&2
|
||||
return 1
|
||||
fi
|
||||
|
||||
set -- ${ETV_VERDICT_POSITIVE_WORDS[@]+"${ETV_VERDICT_POSITIVE_WORDS[@]}"} \
|
||||
${ETV_VERDICT_NEGATIVE_WORDS[@]+"${ETV_VERDICT_NEGATIVE_WORDS[@]}"}
|
||||
total=$#
|
||||
|
||||
for word in "$@"; do
|
||||
index=$((index + 1))
|
||||
word=$(printf '%s' "$word" | tr '[:lower:]' '[:upper:]')
|
||||
if [ "$index" -eq 1 ]; then
|
||||
out="$word"
|
||||
elif [ "$index" -eq "$total" ]; then
|
||||
out="$out or $word"
|
||||
else
|
||||
out="$out, $word"
|
||||
fi
|
||||
done
|
||||
|
||||
printf '%s\n' "$out"
|
||||
}
|
||||
|
||||
# --- Reached-the-end marker. --------------------------------------------------------------------
|
||||
# A top-level `exit` in a SOURCED file terminates the sourcing script where it stands, so no check
|
||||
# placed after the `source` can ever run — the reader returned exit 0 with empty stdout, violating
|
||||
# its own "exactly one classification word" contract. Both consumers therefore source this file in a
|
||||
# SUBSHELL first and require the marker below on stdout; a file that exits early never prints it.
|
||||
# Defining a function is not enough (a truncated file can define every function and still stop
|
||||
# short), so this is the LAST line and it must stay last.
|
||||
etv_verdict_vocabulary_loaded() { printf 'etv-verdict-vocabulary-loaded\n'; }
|
||||
Reference in New Issue
Block a user