fix(review-loop-run): counts parses count-by-severity block with scope + format branches (2026-09-13 fix, 40/40 tests)
Was live via ~/sbin symlink but never committed. Baseline for bug 5352fe72. Claude-Session: https://claude.ai/code/session_01G3SkMRBRWXyiGuMGs8WVas
This commit is contained in:
@@ -39,24 +39,80 @@ report_dir_for() {
|
|||||||
echo "${REVIEW_LOOP_RUN_DIR:-${XDG_RUNTIME_DIR:-/tmp}/review-loop/$(basename "$repo")}"
|
echo "${REVIEW_LOOP_RUN_DIR:-${XDG_RUNTIME_DIR:-/tmp}/review-loop/$(basename "$repo")}"
|
||||||
}
|
}
|
||||||
|
|
||||||
|
# _counts_scope <file> → prints the text of the last "count by severity"
|
||||||
|
# section (heading matching /count.*severity|severity.*count|summary/i, up
|
||||||
|
# to the next heading or EOF), or the last 40 lines if no such heading exists.
|
||||||
|
_counts_scope() {
|
||||||
|
local f="$1"
|
||||||
|
local hdr_line
|
||||||
|
hdr_line=$(grep -inE 'count.*severity|severity.*count|summary' "$f" | tail -1 | cut -d: -f1) || true
|
||||||
|
if [[ -n "$hdr_line" ]]; then
|
||||||
|
awk -v start="$hdr_line" '
|
||||||
|
NR==start { found=1 }
|
||||||
|
found && NR>start && /^#{1,4}[[:space:]]/ { exit }
|
||||||
|
found { print }
|
||||||
|
' "$f"
|
||||||
|
else
|
||||||
|
tail -n 40 "$f"
|
||||||
|
fi
|
||||||
|
}
|
||||||
|
|
||||||
|
# _counts_extract <scope-text> <sev> → last matching value for SEV in scope,
|
||||||
|
# accepting: "SEV: n", "SEV = n", "- SEV: n", "**SEV**: n", "| SEV | n |",
|
||||||
|
# "SEV (n)", "n SEV". Empty if none found.
|
||||||
|
_counts_extract() {
|
||||||
|
local scope="$1" sev="$2" line n
|
||||||
|
# Strip markdown emphasis markers (bold/italic) — they can appear before
|
||||||
|
# or after the colon (e.g. "**SEV:**" as well as "**SEV**:").
|
||||||
|
local clean; clean=$(sed 's/\*//g' <<<"$scope")
|
||||||
|
line=$(grep -inE "\\| *${sev} *\\| *[0-9]+ *\\||${sev}[[:space:]]*[:=][[:space:]]*[0-9]+|${sev}[[:space:]]*\\([0-9]+\\)|[0-9]+[[:space:]]+${sev}\\b" <<<"$clean" | tail -1) || true
|
||||||
|
[[ -z "$line" ]] && { echo ""; return; }
|
||||||
|
if [[ "$line" =~ \|[[:space:]]*${sev}[[:space:]]*\|[[:space:]]*([0-9]+)[[:space:]]*\| ]]; then
|
||||||
|
n="${BASH_REMATCH[1]}"
|
||||||
|
elif [[ "$line" =~ ${sev}[[:space:]]*[:=][[:space:]]*([0-9]+) ]]; then
|
||||||
|
n="${BASH_REMATCH[1]}"
|
||||||
|
elif [[ "$line" =~ ${sev}[[:space:]]*\(([0-9]+)\) ]]; then
|
||||||
|
n="${BASH_REMATCH[1]}"
|
||||||
|
elif [[ "$line" =~ ([0-9]+)[[:space:]]+${sev} ]]; then
|
||||||
|
n="${BASH_REMATCH[1]}"
|
||||||
|
fi
|
||||||
|
echo "${n:-}"
|
||||||
|
}
|
||||||
|
|
||||||
# counts <file> → prints CRITICAL=n HIGH=n MEDIUM=n LOW=n ; exit 4 if C+H>0
|
# counts <file> → prints CRITICAL=n HIGH=n MEDIUM=n LOW=n ; exit 4 if C+H>0
|
||||||
|
# Parses the review's own "count by severity" block first (last such block in
|
||||||
|
# the file); only falls back to counting finding headings (marked "(derived)")
|
||||||
|
# when no count block yields any severity value at all.
|
||||||
counts() {
|
counts() {
|
||||||
local f="$1"
|
local f="$1"
|
||||||
[[ -f "$f" ]] || die "result file not found: $f" 2
|
[[ -f "$f" ]] || die "result file not found: $f" 2
|
||||||
|
shopt -s nocasematch
|
||||||
|
local scope; scope=$(_counts_scope "$f")
|
||||||
|
local -A vals=()
|
||||||
|
local sev found_any=0
|
||||||
|
for sev in CRITICAL HIGH MEDIUM LOW; do
|
||||||
|
local n; n=$(_counts_extract "$scope" "$sev")
|
||||||
|
if [[ -n "$n" ]]; then vals[$sev]="$n"; found_any=1; fi
|
||||||
|
done
|
||||||
|
|
||||||
|
local derived=""
|
||||||
|
if [[ "$found_any" -eq 0 ]]; then
|
||||||
|
derived=" (derived)"
|
||||||
|
for sev in CRITICAL HIGH MEDIUM LOW; do
|
||||||
|
local n
|
||||||
|
n=$(grep -cE "^[[:space:]]*(#+[[:space:]]*)?(\*\*)?${sev}(\*\*)?[[:space:]]*(:|—|-)" "$f") || n=0
|
||||||
|
vals[$sev]="$n"
|
||||||
|
done
|
||||||
|
fi
|
||||||
|
shopt -u nocasematch
|
||||||
|
|
||||||
local out="" total=0
|
local out="" total=0
|
||||||
for sev in CRITICAL HIGH MEDIUM LOW; do
|
for sev in CRITICAL HIGH MEDIUM LOW; do
|
||||||
local n
|
local n="${vals[$sev]:-0}"
|
||||||
n=$(grep -iE "^\| *${sev} *\| *[0-9]+" "$f" | head -1 | sed -E 's/^\| *[A-Za-z]+ *\| *([0-9]+).*/\1/') || true
|
out+="${sev}=${n} "
|
||||||
if [[ -z "$n" ]]; then
|
|
||||||
n=$(grep -iE "^[-*]* *\**${sev}:?\**:? *\**([0-9]+)" "$f" | head -1 | sed -E 's/.*[^0-9]([0-9]+)[^0-9]*$/\1/') || true
|
|
||||||
fi
|
|
||||||
if [[ -z "$n" ]]; then
|
|
||||||
n=$(grep -cE "^#{2,4} .*\b${sev}\b" "$f") || true
|
|
||||||
fi
|
|
||||||
n="${n:-0}"; out+="${sev}=${n} "
|
|
||||||
if [[ "$sev" == "CRITICAL" || "$sev" == "HIGH" ]]; then total=$((total + n)); fi
|
if [[ "$sev" == "CRITICAL" || "$sev" == "HIGH" ]]; then total=$((total + n)); fi
|
||||||
done
|
done
|
||||||
echo "$out"
|
echo "${out% }${derived}"
|
||||||
if [[ "$total" -gt 0 ]]; then return 4; fi
|
if [[ "$total" -gt 0 ]]; then return 4; fi
|
||||||
return 0
|
return 0
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -56,7 +56,14 @@ review-loop-run --help
|
|||||||
|
|
||||||
`status` scans `<dir>/*-r*-stdout.txt`; a file whose last line matches `^exit=` is `done`, else `running`; counts come from `counts`.
|
`status` scans `<dir>/*-r*-stdout.txt`; a file whose last line matches `^exit=` is `done`, else `running`; counts come from `counts`.
|
||||||
|
|
||||||
`counts` parses a result file with three fallbacks: a markdown table row `| CRITICAL | <n>`, a list line `- **CRITICAL:** <n>` / `CRITICAL: <n>`, or (failing both) the number of headings containing `CRITICAL`/`HIGH` — the same for HIGH/MEDIUM/LOW.
|
`counts` parses the review's own count-by-severity block first, never bare numbers scattered through the file (a finding heading like "## Finding 3 — HTTP 404 on ..." or "### MEDIUM: ..." must not be misread as a count):
|
||||||
|
|
||||||
|
1. **Locate the scope.** Find the *last* line matching `/count.*severity|severity.*count|summary/i` and take from that heading up to the next `#`-heading (or EOF). If no such heading exists, the scope is the last 40 lines of the file.
|
||||||
|
2. **Extract per severity, within scope only.** For each of CRITICAL/HIGH/MEDIUM/LOW, take the *last* match in scope of any of: `SEV: n`, `SEV = n`, `- SEV: n`, `**SEV**: n` (markdown emphasis markers around/inside the label are stripped before matching, so `**SEV:**` also matches), `| SEV | n |`, `SEV (n)`, or `n SEV` (word-boundary). A severity with no match in scope is 0 for this pass.
|
||||||
|
3. **Fallback (derived).** Only if scope step 2 finds *no* severity at all (no count block present anywhere), fall back to counting lines that START with the severity word — `^\s*(#+\s*)?(\*\*)?(CRITICAL|HIGH|MEDIUM|LOW)\b` followed by `:`, `—`, or `-` — one count per finding line, never bare numbers elsewhere. Append a literal ` (derived)` marker to the output in this case.
|
||||||
|
4. Missing severity → 0. Output stays `CRITICAL=n HIGH=n MEDIUM=n LOW=n` (plus the optional trailing `(derived)` marker); exit 4 iff CRITICAL+HIGH>0, else 0.
|
||||||
|
|
||||||
|
Fixtures: `tests/fixtures/review-loop-run/{bullet-list,table,inline-bold,heading-numbers,no-count-block}.md` cover, respectively: a bullet-list count block, a markdown-table count block, an inline `**HIGH**: 2` style block, a file with "HTTP 404" and a bare number in headings alongside a correct trailing count-by-severity table (regression case for the 2026-09-13 MEDIUM=404/HIGH=11 mis-parse), and a file with no count block at all (derived fallback from finding headings).
|
||||||
|
|
||||||
## Templates (per `--kind`)
|
## Templates (per `--kind`)
|
||||||
|
|
||||||
|
|||||||
10
tests/fixtures/review-loop-run/bullet-list.md
vendored
Normal file
10
tests/fixtures/review-loop-run/bullet-list.md
vendored
Normal file
@@ -0,0 +1,10 @@
|
|||||||
|
# Review
|
||||||
|
|
||||||
|
Some findings above.
|
||||||
|
|
||||||
|
## Summary
|
||||||
|
|
||||||
|
- CRITICAL: 0
|
||||||
|
- HIGH: 1
|
||||||
|
- MEDIUM: 2
|
||||||
|
- LOW: 0
|
||||||
16
tests/fixtures/review-loop-run/heading-numbers.md
vendored
Normal file
16
tests/fixtures/review-loop-run/heading-numbers.md
vendored
Normal file
@@ -0,0 +1,16 @@
|
|||||||
|
# Review
|
||||||
|
|
||||||
|
## Finding 3 — HTTP 404 on /widgets
|
||||||
|
|
||||||
|
### MEDIUM: endpoint returns wrong status
|
||||||
|
|
||||||
|
Some prose mentioning 42 and HIGH severity inline that is not a real count.
|
||||||
|
|
||||||
|
## Count by severity
|
||||||
|
|
||||||
|
| Severity | Count |
|
||||||
|
|----------|-------|
|
||||||
|
| CRITICAL | 0 |
|
||||||
|
| HIGH | 0 |
|
||||||
|
| MEDIUM | 1 |
|
||||||
|
| LOW | 0 |
|
||||||
5
tests/fixtures/review-loop-run/inline-bold.md
vendored
Normal file
5
tests/fixtures/review-loop-run/inline-bold.md
vendored
Normal file
@@ -0,0 +1,5 @@
|
|||||||
|
# Review
|
||||||
|
|
||||||
|
## Count by severity
|
||||||
|
|
||||||
|
CRITICAL: 0. **HIGH**: 2. MEDIUM: 0. LOW: 0.
|
||||||
15
tests/fixtures/review-loop-run/no-count-block.md
vendored
Normal file
15
tests/fixtures/review-loop-run/no-count-block.md
vendored
Normal file
@@ -0,0 +1,15 @@
|
|||||||
|
# Review
|
||||||
|
|
||||||
|
### CRITICAL: auth bypass on /admin
|
||||||
|
|
||||||
|
Detailed explanation of the bypass.
|
||||||
|
|
||||||
|
### HIGH: missing input validation
|
||||||
|
|
||||||
|
Detailed explanation.
|
||||||
|
|
||||||
|
### HIGH: race condition in worker pool
|
||||||
|
|
||||||
|
Detailed explanation.
|
||||||
|
|
||||||
|
No explicit count-by-severity section is present in this file.
|
||||||
10
tests/fixtures/review-loop-run/table.md
vendored
Normal file
10
tests/fixtures/review-loop-run/table.md
vendored
Normal file
@@ -0,0 +1,10 @@
|
|||||||
|
# Review
|
||||||
|
|
||||||
|
## Count by severity
|
||||||
|
|
||||||
|
| Severity | Count |
|
||||||
|
|----------|-------|
|
||||||
|
| CRITICAL | 0 |
|
||||||
|
| HIGH | 0 |
|
||||||
|
| MEDIUM | 1 |
|
||||||
|
| LOW | 3 |
|
||||||
@@ -58,6 +58,28 @@ printf '## CRITICAL findings\n### C1 CRITICAL x\n### H1 HIGH y\n' > "$TMP/heads.
|
|||||||
out=$("$SCRIPT" counts "$TMP/heads.md"); rc=$?
|
out=$("$SCRIPT" counts "$TMP/heads.md"); rc=$?
|
||||||
assert_exit "heading fallback exits 4" 4 "$rc"
|
assert_exit "heading fallback exits 4" 4 "$rc"
|
||||||
|
|
||||||
|
FIXTURES="$REPO_ROOT/tests/fixtures/review-loop-run"
|
||||||
|
out=$("$SCRIPT" counts "$FIXTURES/bullet-list.md"); rc=$?
|
||||||
|
assert_exit "bullet-list block exits 4" 4 "$rc"
|
||||||
|
assert_contains "bullet-list counts" "CRITICAL=0 HIGH=1 MEDIUM=2 LOW=0" "$out"
|
||||||
|
|
||||||
|
out=$("$SCRIPT" counts "$FIXTURES/table.md"); rc=$?
|
||||||
|
assert_exit "table block exits 0" 0 "$rc"
|
||||||
|
assert_contains "table counts" "CRITICAL=0 HIGH=0 MEDIUM=1 LOW=3" "$out"
|
||||||
|
|
||||||
|
out=$("$SCRIPT" counts "$FIXTURES/inline-bold.md"); rc=$?
|
||||||
|
assert_exit "inline bold exits 4" 4 "$rc"
|
||||||
|
assert_contains "inline bold counts" "CRITICAL=0 HIGH=2 MEDIUM=0 LOW=0" "$out"
|
||||||
|
|
||||||
|
out=$("$SCRIPT" counts "$FIXTURES/heading-numbers.md"); rc=$?
|
||||||
|
assert_exit "heading with HTTP 404 exits 0" 0 "$rc"
|
||||||
|
assert_contains "heading-numbers ignores 404/42, uses own count block" "CRITICAL=0 HIGH=0 MEDIUM=1 LOW=0" "$out"
|
||||||
|
|
||||||
|
out=$("$SCRIPT" counts "$FIXTURES/no-count-block.md"); rc=$?
|
||||||
|
assert_exit "no count block derives, exits 4" 4 "$rc"
|
||||||
|
assert_contains "no count block counts" "CRITICAL=1 HIGH=2 MEDIUM=0 LOW=0" "$out"
|
||||||
|
assert_contains "no count block marked derived" "(derived)" "$out"
|
||||||
|
|
||||||
echo "review-loop-run — status"
|
echo "review-loop-run — status"
|
||||||
printf 'x\nexit=0\n' > "$REVIEW_LOOP_RUN_DIR/t-r1-stdout.txt"
|
printf 'x\nexit=0\n' > "$REVIEW_LOOP_RUN_DIR/t-r1-stdout.txt"
|
||||||
out=$("$SCRIPT" status --name t 2>&1); rc=$?
|
out=$("$SCRIPT" status --name t 2>&1); rc=$?
|
||||||
|
|||||||
Reference in New Issue
Block a user