fix(629): round-3 fixes — fence LENGTH semantics, and stop masking reader failures
review-verdict/h10 Awaiting review verdict for 299e7b2
Review verdict / Set review-verdict status (pull_request) Successful in 3s
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 12s
PR Gates / Docs update reminder (pull_request) Successful in 17s
PR Gates / decisions lifecycle (pull_request) Successful in 25s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 8m10s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 10s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 18s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 17m21s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 22m25s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
review-verdict/h10 Awaiting review verdict for 299e7b2
Review verdict / Set review-verdict status (pull_request) Successful in 3s
PR Gates / CI image pin matches docker/ci (pull_request) Successful in 12s
PR Gates / Docs update reminder (pull_request) Successful in 17s
PR Gates / decisions lifecycle (pull_request) Successful in 25s
Build ErsatzTV Image / Build & test (.NET) (pull_request) Successful in 8m10s
Build ErsatzTV Image / API docs in sync (OpenAPI + endpoint index) (pull_request) Successful in 10s
Build ErsatzTV Image / Formatting (changed .cs conform to .editorconfig) (pull_request) Successful in 18s
Build ErsatzTV Image / Functional E2E (curl + UI contracts) (pull_request) Successful in 17m21s
Build ErsatzTV Image / EF migration integrity (SQLite + MySql) (pull_request) Successful in 22m25s
Build ErsatzTV Image / Build & push image (amd64) (pull_request) Has been skipped
Third review round, third set of real findings. Both reproduced before fixing.
1. High: markdown closes a fence only with N-or-more of the SAME marker it was opened with, so a
```` block legitimately CONTAINS a ``` line as content. Toggling on any 3+ marker left the
fence at that inner line and graded the verdict below it as a real approval:
````text / ``` / Review-verdict: MERGEABLE @ <head> / ```` -> positive
Now tracks the opening marker's character and length; a shorter or different marker while a
fence is open is content, so it neither closes the fence nor escapes it.
2. Medium: `awk ... | grep ... || true` flattened "no match" (grep rc 1, normal) together with a
real tool failure (rc >= 2). A failing reader produced no verdict lines at all — `absent` —
silently discarding a real BLOCKED verdict. awk and grep are now checked separately, and only
"no match" is tolerated.
Also fixed while writing (2): `[ rc = 0 ] && printf` as the loop body's LAST command would leave
the subshell exiting 1 whenever the newest comment carried no verdict, which the rc check would
then report as a failure to read comment bodies — an ordinary PR reading as broken. Uses an `if`.
169 tests. Both findings mutation-verified: restoring the naive fence toggle fails all four
longer-fence cases, restoring `|| true` fails the failing-grep case, control green. Verified the
ordinary shapes still work: plain ``` and ~~~ fences and lang-tagged fences still stripped, an
unclosed fence still swallows, a real verdict beside a fenced example still counts, and a fenced
positive alongside a real BLOCKED still classifies negative.
refs #629
Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Decisions-Edit: yes
This commit is contained in:
@@ -107,10 +107,30 @@ verdicts=$(printf '%s\n' "$encoded" | while IFS= read -r encoded_body; do
|
||||
# A body that fails to decode is fatal, not skippable — skipping one could drop the only BLOCKED
|
||||
# verdict on the PR. 3 is distinct from the exit-2 paths above so the caller sees where it broke.
|
||||
body=$(printf '%s' "$encoded_body" | jq -r '.' 2>/dev/null) || exit 3
|
||||
printf '%s\n' "$body" | awk '
|
||||
/^[[:space:]]*(```|~~~)/ { fence = !fence; next }
|
||||
!fence { print }
|
||||
' | grep -iE '^[[:space:]]*review-verdict:' || true
|
||||
# Fence tracking follows markdown: a fence opened with N markers is closed only by N-or-more of the
|
||||
# SAME character. A naive "any line starting with 3 toggles" exits a ```` block at the first ```
|
||||
# LINE INSIDE IT — which is legitimate content — and the verdict below it then counts as real.
|
||||
# A shorter or different marker while a fence is open is content, so it neither closes nor prints.
|
||||
outside=$(printf '%s\n' "$body" | awk '
|
||||
{
|
||||
if (match($0, /^[[:space:]]*(`{3,}|~{3,})/)) {
|
||||
m = substr($0, RSTART, RLENGTH); gsub(/[[:space:]]/, "", m)
|
||||
ch = substr(m, 1, 1); len = length(m)
|
||||
if (!fence) { fence = 1; fch = ch; flen = len; next }
|
||||
else if (ch == fch && len >= flen) { fence = 0; next }
|
||||
}
|
||||
if (!fence) print
|
||||
}') || exit 4
|
||||
# `grep` exits 1 for "no match" (normal) and >=2 for a real error. `|| true` flattened both into
|
||||
# success, so a failing reader silently produced no verdicts — `absent` — and dropped a real
|
||||
# BLOCKED verdict. Only "no match" may be tolerated.
|
||||
found=$(printf '%s\n' "$outside" | grep -iE '^[[:space:]]*review-verdict:')
|
||||
grep_rc=$?
|
||||
[ "$grep_rc" -le 1 ] || exit 5
|
||||
# An `if`, not `[ ... ] && printf`: the latter is the loop body's last command, so a final comment
|
||||
# with no verdict would leave the SUBSHELL exiting 1 and the rc check below would report a reader
|
||||
# failure on a perfectly ordinary PR.
|
||||
if [ "$grep_rc" -eq 0 ]; then printf '%s\n' "$found"; fi
|
||||
done)
|
||||
verdict_rc=$?
|
||||
if [ "$verdict_rc" -ne 0 ]; then
|
||||
|
||||
Reference in New Issue
Block a user