fix(788): one declarative H10 verdict vocabulary, derived by both sides #846

Merged

timothy merged 6 commits from fix/788-shared-verdict-vocabulary into main 2026-08-26 22:58:10 +02:00

Conversation 1 Commits 6 Files Changed 14

+1053 -32

timothy commented 2026-08-26 22:36:07 +02:00

Owner

Copy Link

Copy Source

Fixes #788.

The H10 verdict vocabulary lived in two hand-written shell copies — the case arms of
post-review-verdict.sh (write: word → commit-status state) and the POS_RE/NEG_RE regexes of
check-review-verdict.sh (read: comment → classification for the merge-consent hook). Nothing held
them together but a comment, and that comment had already gone stale once. A word the writer treats
as positive but the classifier does not sends review-verdict/h10 green while the hook still
denies.

scripts/lib/review-verdict-vocabulary.sh now declares the words once, as two bash arrays, and both
sides derive: a membership test for the writer, an ERE alternation body for the reader, and the
operator-facing list for the usage banner and the unknown-word error (both of which were themselves
further copies). Neither script enumerates a verdict word any more.

Only the WORD SET moved. The grammar stays in check-review-verdict.sh — the column-0 anchor, the
whole-word boundary, the anchored @ <sha> field. Every #629 false-open was in the grammar, and
none of it belongs in a word list.

No parity test. #774 shipped one and withdrew it after six review rounds, each finding another
shell construction that escaped the regex extractor or reddened it on a correct tree. The proof here
is behavioural: add a word to a sandbox copy of the one declaration and require it to reach both
sides. Graded MUTATION, with the clause declared in mutation_manifest.py — the harness restores
the pre-#788 hardcoded POS_RE on every run and requires the proof to go red.

Review history — this PR shipped a fail-open and it was caught

Four rounds of cold adversarial review, each from a fresh review-only brief in an isolated worktree.

Round 1 found a real fail-open in the first commit, reproduced before fixing: ${#arr[@]} is
nounset-safe only for a declared-empty array; on an unset name or a scalar it is fatal. Under
set -u that error inside a function aborts the function but not the script, and in the
if ! validate; then exit 2; fi form neither branch runs — so validation was skipped. The reader
deliberately has no set -e, so unvalidated words reached the regex: with .* in the positive list
and the negative list written as a scalar, an explicit BLOCKED @ <head> classified positive,
exit 0, with the only signal on a stderr the hook discards.

The fix replaces the control-flow gate with a data dependency: validation sets a sentinel on its
last line and the derived views refuse without it. A gate can be skipped by an abort; a value never
assigned cannot be read. Plus a declare -p array assertion so the abort cannot happen, and a
subshell probe requiring an end-of-file marker (a top-level exit in a sourced file terminates the
sourcing script, so no check after the source can run).

Rounds 2, 3 and 4 found no fail-open — the core was attacked with sentinel forgery, environment
poisoning, declare -p evasion across every declaration form on bash 5.3 and 3.2, path/symlink
resolution, probe TOCTOU, regex injection, and multi-thousand-payload differential fuzzing. What
they kept finding was inaccurate prose: every commit in this PR carried at least one false
statement, including one introduced while correcting false statements.

The count of prose vocabulary copies was wrong three times running (one → two → two, while a third
sat four lines away). It is now deleted rather than corrected a fourth time, and the record
carries the derivation instead — the earlier sweeps all anchored on MERGEABLE, so a negative-only
enumeration like BLOCKED/NOT-MERGEABLE was invisible to every one of them. A count is a second copy
of the list and rots like one, which is the argument the record itself is built on.

Along the way: CLAUDE.md and release.review-verdict-gate both enumerated the vocabulary without
LGTM, a word the code has accepted since #629 — so the record a reader consults for this gate was
teaching a vocabulary missing a live word. Corrected.

Verification

• 989 passed, 2 skipped (pytest scripts/tests); ruff, ruff format --check and shellcheck clean;
  catalog regenerated; decisions-validate: OK.
• Behavioural equivalence: 4788-case differential fuzz of the new classifier against
  origin/main's — 0 divergences, including every #629 false-open case. An independent reviewer
  ran a separate 2612-payload fuzz, also 0.
• Every malformation fails closed on both sides (missing, unreadable, truncated, syntax error,
  top-level exit, unset array, scalar list, duplicate word, empty list, regex metacharacter, glob):
  reader exit 2, writer exit 1 with nothing posted — verified against a POST-logging curl stub.
• The sentinel is proved load-bearing: deleting all three guards turns three tests red, where
  previously 150 tests stayed green with it removed. The comment states precisely which of the three
  copies has a behavioural alone-deletion red and which are defence-in-depth behind set -e.

Docs

docs/ci-cd.md (Review-verdict gate), a new release.verdict-vocabulary-shared record + regenerated
catalog, docs/guard-inventory.md (new row + summary counts), docs/remote-state-inventory.md.

Known residual, stated not implied

The merge-consent hook and a few docs name the words in prose. Those copies are inert — nothing
outside the library classifies a word — and the record carries the sweep that re-derives them rather
than a number. The library is located relative to ${BASH_SOURCE[0]} without resolving symlinks, so
a script reached through a symlink looks beside the link; that fails closed and is recorded.

Fixes #788. The H10 verdict vocabulary lived in two hand-written shell copies — the `case` arms of `post-review-verdict.sh` (write: word → commit-status state) and the `POS_RE`/`NEG_RE` regexes of `check-review-verdict.sh` (read: comment → classification for the merge-consent hook). Nothing held them together but a comment, and that comment had already gone stale once. A word the writer treats as positive but the classifier does not sends `review-verdict/h10` green while the hook still denies. `scripts/lib/review-verdict-vocabulary.sh` now declares the words once, as two bash arrays, and both sides derive: a membership test for the writer, an ERE alternation body for the reader, and the operator-facing list for the usage banner and the unknown-word error (both of which were themselves further copies). **Neither script enumerates a verdict word any more.** Only the WORD SET moved. The grammar stays in `check-review-verdict.sh` — the column-0 anchor, the whole-word boundary, the anchored `@ <sha>` field. Every #629 false-open was in the grammar, and none of it belongs in a word list. **No parity test.** #774 shipped one and withdrew it after six review rounds, each finding another shell construction that escaped the regex extractor or reddened it on a correct tree. The proof here is behavioural: add a word to a sandbox copy of the one declaration and require it to reach both sides. Graded `MUTATION`, with the clause declared in `mutation_manifest.py` — the harness restores the pre-#788 hardcoded `POS_RE` on every run and requires the proof to go red. ## Review history — this PR shipped a fail-open and it was caught Four rounds of cold adversarial review, each from a fresh review-only brief in an isolated worktree. **Round 1 found a real fail-open in the first commit**, reproduced before fixing: `${#arr[@]}` is nounset-safe only for a *declared-empty* array; on an unset name or a scalar it is fatal. Under `set -u` that error inside a function aborts the function but not the script, and in the `if ! validate; then exit 2; fi` form **neither branch runs** — so validation was skipped. The reader deliberately has no `set -e`, so unvalidated words reached the regex: with `.*` in the positive list and the negative list written as a scalar, an explicit `BLOCKED @ <head>` classified **`positive`, exit 0**, with the only signal on a stderr the hook discards. The fix replaces the control-flow gate with a **data dependency**: validation sets a sentinel on its last line and the derived views refuse without it. A gate can be skipped by an abort; a value never assigned cannot be read. Plus a `declare -p` array assertion so the abort cannot happen, and a subshell probe requiring an end-of-file marker (a top-level `exit` in a sourced file terminates the sourcing script, so no check after the `source` can run). **Rounds 2, 3 and 4 found no fail-open** — the core was attacked with sentinel forgery, environment poisoning, `declare -p` evasion across every declaration form on bash 5.3 and 3.2, path/symlink resolution, probe TOCTOU, regex injection, and multi-thousand-payload differential fuzzing. What they kept finding was **inaccurate prose**: every commit in this PR carried at least one false statement, including one introduced while correcting false statements. The count of prose vocabulary copies was wrong three times running (one → two → two, while a third sat four lines away). It is now **deleted rather than corrected a fourth time**, and the record carries the derivation instead — the earlier sweeps all anchored on `MERGEABLE`, so a negative-only enumeration like `BLOCKED/NOT-MERGEABLE` was invisible to every one of them. A count is a second copy of the list and rots like one, which is the argument the record itself is built on. Along the way: `CLAUDE.md` and `release.review-verdict-gate` both enumerated the vocabulary **without `LGTM`**, a word the code has accepted since #629 — so the record a reader consults for this gate was teaching a vocabulary missing a live word. Corrected. ## Verification - `989 passed, 2 skipped` (`pytest scripts/tests`); ruff, `ruff format --check` and shellcheck clean; catalog regenerated; `decisions-validate: OK`. - **Behavioural equivalence: 4788-case differential fuzz** of the new classifier against `origin/main`'s — **0 divergences**, including every #629 false-open case. An independent reviewer ran a separate 2612-payload fuzz, also 0. - **Every malformation fails closed on both sides** (missing, unreadable, truncated, syntax error, top-level `exit`, unset array, scalar list, duplicate word, empty list, regex metacharacter, glob): reader exit 2, writer exit 1 with **nothing posted** — verified against a POST-logging curl stub. - The sentinel is proved load-bearing: deleting all three guards turns three tests red, where previously 150 tests stayed green with it removed. The comment states precisely which of the three copies has a behavioural alone-deletion red and which are defence-in-depth behind `set -e`. ## Docs `docs/ci-cd.md` (Review-verdict gate), a new `release.verdict-vocabulary-shared` record + regenerated catalog, `docs/guard-inventory.md` (new row + summary counts), `docs/remote-state-inventory.md`. ## Known residual, stated not implied The merge-consent hook and a few docs name the words in prose. Those copies are inert — nothing outside the library classifies a word — and the record carries the sweep that re-derives them rather than a number. The library is located relative to `${BASH_SOURCE[0]}` without resolving symlinks, so a script reached through a symlink looks beside the link; that fails closed and is recorded.

timothy added 6 commits 2026-08-26 22:36:08 +02:00

fix(788): one declarative H10 verdict vocabulary, derived by both sides ... dd9cd113c3

The verdict words lived in two hand-written shell copies — the `case` arms of
`post-review-verdict.sh` (write: word -> commit-status state) and the
`POS_RE`/`NEG_RE` regexes of `check-review-verdict.sh` (read: comment ->
classification for the merge-consent hook). Nothing held them together but a
comment, and that comment had already gone stale once. A word the writer treats
as positive but the classifier does not sends `review-verdict/h10` green while
the hook still denies.

#774 tried to hold them together with a parity test that scraped both
vocabularies out of shell source with regexes. Six cold-review rounds each found
another construction that escaped the extractor (`SHIP*)`; an unquoted
`SHIP-IT) state=success`) or reddened it on a correct tree (a `<word>) state=`
in a heredoc; a column-zero `esac` in a string truncating the scoped match). It
was withdrawn rather than patched a seventh time. So this removes the
duplication instead of detecting it.

`scripts/lib/review-verdict-vocabulary.sh` declares the two word arrays once and
exposes three derived views: a membership test for the writer, an alternation
body for the reader, and the operator-facing list for the "unknown verdict"
message and the usage banner (both of which were themselves third copies).
Neither script enumerates a word any more.

Only the WORD SET moved. The grammar stays in `check-review-verdict.sh` — the
column-0 anchor, the whole-word boundary, the anchored `@ <sha>` field. Every
#629 false-open was in the grammar, and none of it belongs in a word list.

Validation is the security property, not hygiene. The words are interpolated
into an ERE, so a `.*` in the positive list would make `POS_RE` match every
verdict-shaped line and classify an explicit BLOCKED as `positive`. Words are
restricted to `[a-z][a-z-]*`, and a vocabulary that is missing, unreadable,
duplicated across polarities or empty is fatal — exit 2 for the reader (callers
fail closed on it), exit 1 for the writer, having posted nothing.

The arrays are arrays rather than whitespace-separated strings because splitting
a string needs an unquoted expansion, and an unquoted expansion also globs: the
first draft, handed `.*`, reported on `.claude`. That makes validation depend on
the cwd and fails open — a `*` expands to the filenames beside it, and
`scripts`/`docs` both match `[a-z][a-z-]*` and would have entered the vocabulary
as real verdict words. Caught by the negative control, not by review.

The proof is behavioural, never textual: add a word to a sandbox copy of the one
declaration and require it to reach both sides. Graded MUTATION, with the clause
declared in `mutation_manifest.py` — the harness restores the pre-#788 hardcoded
`POS_RE` on every run and requires the proof to go red, so a refactor that
quietly reintroduces a second copy cannot leave the suite green.

Known residual, deliberately not fixed: `pretooluse-merge-consent.sh` names the
words in one operator-facing message string. It classifies nothing (it delegates
to `check-review-verdict.sh`), so it is a prose copy, not a second
implementation.

Verification: 980 passed / 2 skipped (`pytest scripts/tests`), ruff clean,
shellcheck clean, catalog regenerated.

Refs: #788
Decisions-Edit: yes
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>

fix(788): gate the vocabulary on a sentinel, not on a skippable if ... 99caae7615

Cold adversarial review found a fail-open in the first commit, reproduced here
before fixing: with `.*` in the positive list and the negative list written as a
scalar, `check-review-verdict.sh` classified an explicit `BLOCKED @ <head>` as
`positive`, exit 0. The merge-consent hook auto-grants on exactly that, and it
discards the classifier's stderr, so the only signal was invisible.

Root cause, measured rather than reasoned:

  * `${#arr[@]}` is nounset-safe only for a DECLARED-EMPTY array. On an unset
    name or a scalar it is a fatal unbound-variable error.
  * Under `set -u` that error inside a FUNCTION aborts the function, not the
    script — and in the `if ! validate; then exit 2; fi` form NEITHER branch is
    then taken, so the gate is skipped and execution continues.
  * `check-review-verdict.sh` deliberately runs without `set -e`, so the words
    reached the regex unvalidated. The write side has `set -e` and did die, so
    the asymmetry left the hole on precisely the side that authorizes merges.

The single-fault case needed no metacharacter and is the realistic wrong edit:
writing the list as a string degraded every negative verdict from `negative` to
`unknown` — DENY silently downgraded to ASK — with no non-zero exit anywhere.

A control-flow gate can be skipped by an abort; a value that was never assigned
cannot be read. So enforcement is now a DATA dependency: validation sets
`ETV_VERDICT_VOCABULARY_OK=1` as the last statement of its successful path, and
`etv_verdict_class`/`etv_verdict_alternation`/`etv_verdict_words_display` refuse
without it. The sentinel is reset to 0 at load time, so an inherited environment
variable cannot forge it. Array-ness is asserted with `declare -p` BEFORE any
`${#arr[@]}`, so the abort cannot occur in the first place — kept alongside the
sentinel for the accurate message, not as a second gate.

Separately, a top-level `exit` in a sourced library terminates the sourcing
script at the `source` line, so no check written after it can run: the reader
returned exit 0 with EMPTY stdout, breaking its "exactly one classification
word" contract. Both consumers now probe-source the library in a subshell and
require an end-of-file marker before sourcing it for real; that also catches a
syntax error and a truncated file.

All six failure modes now fail closed (reader exit 2, writer exit 1 having
posted nothing), and the shipped words classify exactly as before. Four of the
six new tests were witnessed RED against the unfixed predecessor 2d389a5bf; the
other two (unset array, truncated library) pin behaviour that already held and
are labelled as such rather than presented as fixes.

Also from the review: prose in the decision record, `docs/ci-cd.md` and two
in-script comments claimed validation was "run above, fatal" — the false premise
that shipped the hole. Corrected to name the sentinel as what enforces it. The
unresolved-symlink location dependency is recorded as a known limitation
(fail-closed today). De-flaked the unreadable-file test for a root runner, and
fixed a stale `check-review-verdict.sh:212` line reference plus a comment still
describing #788 as future work.

Not done, deliberately: no third guard on the both-patterns-match case. The
validator rejects a word in both lists, and test_check_review_verdict.py already
pins the dangerous direction; a third overlapping guard is the shape where
deleting any one leaves the suite green.

Verification: 987 passed / 2 skipped, ruff clean, shellcheck clean, catalog
regenerated, decisions-validate OK.

Refs: #788
Decisions-Edit: yes
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>

fix(788): prove the sentinel is load-bearing, and correct three claims ... 16070d40d3

Second cold review could not break the fix — no fail-open under sentinel
forgery, `declare -p` evasion across 17 declaration forms on bash 5.3 and 3.2,
probe-source attacks, or a 19-case grammar regression battery. What it broke was
the EVIDENCE.

**The sentinel was proved by nothing.** Deleting all three
`ETV_VERDICT_VOCABULARY_OK` guards left 150 tests green. The reason is the other
half of the same fix: `etv_verdict__is_array` removes the `set -u` abort, so
validation now returns 1 cleanly and the ordinary `if ! validate; then exit 2;
fi` fires — meaning every refusal test passes without the sentinel ever
executing. Two guards on one condition, where deleting either leaves the suite
green, is #685 exactly, cited in this test file's own docstring.

So the sentinel is now proved by disarming the OTHER guard first. With the array
assertion removed the abort route reopens, the `if` is skipped, and the sentinel
is the only thing between a malformed word list and the regex:

  * with the array assertion disarmed, the sentinel alone still refuses (exit 2,
    and the assertion requires the sentinel's OWN diagnostic, not merely some
    refusal)
  * removing BOTH reproduces the original fail-open — `positive`, exit 0 — which
    is the control that stops the first test from proving only "it refuses"
  * an inherited `ETV_VERDICT_VOCABULARY_OK=1` still cannot carry a malformed
    vocabulary through, tested with the abort route open

Witnessed: deleting all three guards now turns those three tests RED.

Three claims corrected, all measured:

  * `check-review-verdict.sh:212` -> the previous commit "fixed" this to 279,
    which is also wrong; the line is 284. Found by grep this time rather than by
    counting, in a docstring specifically about measuring rather than reasoning.
  * The record said the merge-consent hook names the words in ONE message
    string. There are two — and the one it excused had ALREADY drifted, omitting
    LGTM from the vocabulary this record exists to unify. The hook comment is
    corrected and the residual is restated as two, with the honest note that
    prose copies drift too and are tolerated only because they are inert.
  * Two test names claimed more than the tests observed
    (`..._cannot_be_PRE_SET_from_the_environment` passed with the sentinel
    deleted; `..._still_sets_the_sentinel_...` cannot see the sentinel). Both
    replaced by tests that isolate what they name.

Also: the usage banner printed the vocabulary lower-case while the unknown-word
error printed it upper-case, so an operator saw two spellings of one list.

Rebased onto origin/main (40a3232d9) — #841 landed from a parallel session while
this branch was in review, and the pre-rebase diff showed that PR's files as
deletions.

Verification: 989 passed / 2 skipped, ruff clean, shellcheck clean (the two
SC1090s on the hook are pre-existing on main), catalog regenerated,
decisions-validate OK.

Refs: #788
Decisions-Edit: yes
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>

fix(788): delete the count instead of correcting it a fourth time ... 4dd15523c9

Round-3 review found no fail-open — it attacked the core hard (inherited
sentinel, env override, missing/truncated/exiting library, regex injection,
scalar list, both bash 5.3 and 3.2, a 2612-payload differential fuzz against
origin/main with 0 diffs, a 12-mode write-side matrix with 0 posts on failure)
and everything failed closed. What it broke was accuracy, again.

**The count of prose vocabulary copies was wrong for the third time.** One draft
said one string, the next said two, the third said two while a third string sat
four lines away in the same file. Each reported the copies someone had noticed.
The sweeps behind them anchored their pattern on `MERGEABLE`, so an enumeration
of the negative words alone — `(BLOCKED/NOT-MERGEABLE)` — was invisible to every
one of them.

So the count is gone rather than corrected. A count is a second copy of the list
and rots exactly like one, which is the argument this whole record is built on;
writing a fourth number would be making the record's own mistake inside the
record. The residual paragraph now carries the derivation instead — a sweep
matching a run of any two adjacent vocabulary words in either polarity, which is
what the earlier patterns could not see.

Corrected with it: `CLAUDE.md` and `release.review-verdict-gate` (twice, so the
catalog too) both enumerated the vocabulary without `LGTM`, a word the code has
accepted since #629. The record a reader consults for this gate was teaching a
vocabulary missing a live word.

**The sentinel proof also overclaimed.** The comment said "delete one gate ALONE
and require a red", and that is true of exactly one of the three sentinel guards.
Measured: deleting the READ side's (`etv_verdict_alternation`) reddens the three
behavioural tests; deleting either write-side copy reddens only the `count == 3`
tripwire. The write side's two are defence-in-depth — `post-review-verdict.sh`
has `set -e`, so the abort that defeats the reader kills the writer outright
(rc=1, nothing posted). They are kept, because the writer should not depend on
`set -e` staying, but their proof is a COUNT and the comment now says so.

Correcting forward rather than rewriting history: the previous commit's message
claims "the two SC1090s on the hook are pre-existing". There is ONE, at
`pretooluse-merge-consent.sh:51`, and only one dynamic-source construct exists in
the file — the earlier count came from grepping shellcheck output, which repeats
the code on its "For more information" line.

Rebased onto ba6a4b08a; #842 and #843 landed from parallel sessions during
review.

Verification: 989 passed / 2 skipped, ruff clean, shellcheck clean, catalog
regenerated, decisions-validate OK, and the documented sweep command executed.

Refs: #788
Decisions-Edit: yes
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>

fix(788): withdraw the completeness claim instead of widening the pattern again ... 87d2bd8450

Round-4 review found no fail-open — 7560-payload differential fuzz against
origin/main with 0 diffs, 28 hostile-vocabulary cases, environment poisoning,
and a 34-case write-side matrix, all fail-closed — and verified every other
claim in the previous commit exactly. It found one thing, and it is the same
thing for the fourth time.

The derivation that replaced the rotting count could not see the generated
catalog. `docs/decisions/README.md` is a Markdown table, so it escapes its pipes
(`MERGEABLE\|APPROVED\|...`), and the pattern required the separator to follow
the word directly. That site is one this branch had just repaired, and the sweep
reported it clean — while the sentence beneath asserted the pattern "matches a
run of any two adjacent vocabulary words, in either polarity".

Widening the separator to `\\?[|/]` finds it, and that is done. But widening
alone would be the fifth attempt to assert a completeness nobody has
established: three counts, then a pattern, now a wider pattern. The
comma-separated list inside `etv_verdict_words_display` is still invisible to
it, and will stay invisible, because a string predicate over prose does not
converge — the same finding #774 recorded for a regex over shell source
(`testing.guard-derives-population-from-source`), which is the record this one
cites.

So the completeness claim is withdrawn rather than re-asserted. The record now
says what the sweep actually does — finds separator-joined enumerations, is
better than a number because it can be re-run, and is NOT evidence that no other
copy exists — and names the catalog omission as the worked example of why. The
safety property never rested on it: nothing outside `review-verdict-vocabulary.sh`
classifies a word, which is what makes the prose copies inert.

Also: the sweep matched its own record's text, so two of its ten hits were the
tool looking at itself while a real site was missing. The record now excludes
itself, and the command was run verbatim as a reader would — it returns nine
sites, all real, including the catalog.

Plus a grammar slip in a comment added two commits ago
(`check-review-verdict.sh`): the sentence lists the anchors and the boundary, but
read as a singular subject with a plural verb.

Verification: 989 passed / 2 skipped, ruff clean, shellcheck clean, catalog
regenerated, decisions-validate OK, 0 commits behind origin/main.

Refs: #788
Decisions-Edit: yes
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>

docs(788): two precision fixes in the vocabulary record ... e66196fd24

Round 5 returned MERGEABLE with no must-fix items. These are its two nits, taken
because this branch's failure mode has been imprecise prose and leaving a known
imprecision to save a round would be the same trade that produced the previous
five.

  * The comma-separated enumeration the sweep cannot see is in the doc COMMENT
    above `etv_verdict_words_display`, not inside it — the body builds its list
    from the data. Verified: the comment is at review-verdict-vocabulary.sh:195,
    the function opens at :198.
  * The separator parenthetical omitted `\/`, which `(\\?[|/])` also accepts.
    That was an understatement rather than an overclaim, and it is the one
    direction this record can afford, but it is still not what the pattern does.
    Verified by feeding `MERGEABLE\/APPROVED` to the pattern: it matches.

No executable line changed. The sweep still runs verbatim and still returns nine
real sites including the catalog; catalog regenerated; decisions-validate OK;
989 passed / 2 skipped.

Refs: #788
Decisions-Edit: yes
Co-authored-by: Timothy <timothy@noreply.gitea.tblindustries.be>

timothy commented 2026-08-26 22:36:52 +02:00

Author

Owner

Copy Link

Copy Source

Review-verdict: MERGEABLE @ e66196f

Six rounds of cold adversarial review, each from a fresh review-only brief in an isolated worktree; the author did not review their own work.

Round 1 found a REAL FAIL-OPEN in the first commit and it is fixed: ${#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 validation was skipped and, on the reader (deliberately no set -e), an explicit BLOCKED @ <head> classified positive, exit 0. Enforcement is now a data dependency (a sentinel set on validation's last line, required by the derived views) rather than a skippable control-flow gate.

Rounds 2-6 found no fail-open. The core was attacked with sentinel forgery, environment poisoning, declare -p evasion across every declaration form on bash 5.3 and 3.2, path/symlink resolution, probe-source TOCTOU, regex injection, and differential fuzzing (4788 payloads by the author, 2612 and 7560 by independent reviewers) — zero divergences from origin/main's grammar in every run. Every malformation fails closed on both sides: reader exit 2, writer exit 1 with nothing posted, verified against a POST-logging curl stub.

What the later rounds kept finding was inaccurate prose: every commit carried at least one false statement, three of them variants of asserting an unestablished completeness. The count of prose vocabulary copies was wrong three times running and is now DELETED rather than corrected a fourth time, replaced by a re-runnable sweep whose completeness claim is explicitly withdrawn.

Round 6 verdict: MERGEABLE, no findings — the first commit on the branch where the reviewer could not find an inaccurate statement.

Verification on this head: 989 passed / 2 skipped, ruff clean, shellcheck clean, catalog regenerated, decisions-validate OK.

Review-verdict: MERGEABLE @ e66196f Six rounds of cold adversarial review, each from a fresh review-only brief in an isolated worktree; the author did not review their own work. Round 1 found a REAL FAIL-OPEN in the first commit and it is fixed: `${#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 validation was skipped and, on the reader (deliberately no `set -e`), an explicit `BLOCKED @ <head>` classified `positive`, exit 0. Enforcement is now a data dependency (a sentinel set on validation's last line, required by the derived views) rather than a skippable control-flow gate. Rounds 2-6 found no fail-open. The core was attacked with sentinel forgery, environment poisoning, `declare -p` evasion across every declaration form on bash 5.3 and 3.2, path/symlink resolution, probe-source TOCTOU, regex injection, and differential fuzzing (4788 payloads by the author, 2612 and 7560 by independent reviewers) — zero divergences from origin/main's grammar in every run. Every malformation fails closed on both sides: reader exit 2, writer exit 1 with nothing posted, verified against a POST-logging curl stub. What the later rounds kept finding was inaccurate prose: every commit carried at least one false statement, three of them variants of asserting an unestablished completeness. The count of prose vocabulary copies was wrong three times running and is now DELETED rather than corrected a fourth time, replaced by a re-runnable sweep whose completeness claim is explicitly withdrawn. Round 6 verdict: MERGEABLE, no findings — the first commit on the branch where the reviewer could not find an inaccurate statement. Verification on this head: 989 passed / 2 skipped, ruff clean, shellcheck clean, catalog regenerated, decisions-validate OK.

timothy merged commit 469d19852c into main 2026-08-26 22:58:10 +02:00

timothy deleted branch fix/788-shared-verdict-vocabulary 2026-08-26 22:58:12 +02:00

timothy referenced this pull request 2026-08-26 22:59:18 +02:00

One shared verdict vocabulary instead of two shell copies plus a parity test #788

Sign in to join this conversation.

Reviewers

No Reviewers

Labels

Clear labels

ad-hoc

One-off / ad-hoc work not tracked by a dedicated issue

api

REST API / HTTP endpoints

bug

Something isn't working

ci-cd

Build, test, deploy pipeline

content

Channel content / schedules / playlists

dependencies

Dependency updates (Renovate)

enhancement

New feature or improvement

frontend

ChicoryTV React SPA frontend

in-progress

Claimed by an active session — do not pick up

jellyfin

Jellyfin tuner / IPTV integration

parked

Excluded from automatic queue pickup; work only when explicitly selected

priority: high

priority: low

priority: medium

review

Adversarial review finding

security

Security / vulnerability fix

No labels

Milestone

No items

No Milestone

Projects

Clear projects

No projects

Assignees

Clear assignees

renovate timothy (Timothy)

No Assignees

1 Participants

Notifications

Subscribe

Due Date

No due date set.

Dependencies

No dependencies set.

Reference: timothy/ersatztv#846