diff --git a/.github/workflows/safety-checks-tests.yml b/.github/workflows/safety-checks-tests.yml new file mode 100644 index 000000000..a4cde0642 --- /dev/null +++ b/.github/workflows/safety-checks-tests.yml @@ -0,0 +1,79 @@ +# Quality gate for scripts/lib/safety-checks.sh — the sourceable PR-safety +# signal helpers backing the Layer 1 spam-pr-guard scorer (epic #1200, +# story #1202). +# +# Scope: tests sc_description_missing, which replaces the naive keyword grep +# that let an unfilled PR template score 0/5 missing — the regression that +# allowed combsmith/.github-private#1976 through automation. +# +# Pattern mirrors .github/workflows/agents-md-rules-tests.yml: pinned +# actions/checkout, path-filtered triggers, read-only token, bats runner. +# ShellCheck for the lib itself is covered by this workflow's bats "passes +# shellcheck" case AND by ci.yml's org-wide ShellCheck job over scripts/**. + +name: Safety Checks Tests + +on: + pull_request: + paths: + - 'scripts/lib/safety-checks.sh' + - 'tests/test_safety_checks.bats' + - 'tests/fixtures/safety-checks/**' + - '.github/workflows/safety-checks-tests.yml' + push: + branches: + - main + paths: + - 'scripts/lib/safety-checks.sh' + - 'tests/test_safety_checks.bats' + - 'tests/fixtures/safety-checks/**' + - '.github/workflows/safety-checks-tests.yml' + +permissions: {} + +# github.sha scopes the concurrency group per-commit, not per-ref: a rapid +# push sequence then cancels only its own in-flight run, so the LATEST +# commit always keeps its own test result (otherwise a push-then-push race +# cancels this run against the newer sha and the merged head has no test +# evidence of its own). +concurrency: + group: safety-checks-tests-${{ github.ref }}-${{ github.sha }} + cancel-in-progress: true + +jobs: + test: + name: Validate PR-safety signal helpers + runs-on: ubuntu-latest + timeout-minutes: 10 + permissions: + contents: read + steps: + - name: Checkout repository + uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + fetch-depth: 1 + # Read-only test job — never pushes; don't leave the token in git config. + persist-credentials: false + + # Bounded retries so a transient apt mirror / DNS blip retries instead of + # failing the quality gate — same shape as .github/workflows/ci-tests.yml + # (the fleet convention for apt-based installs, issue #935 class). + - name: Install bats and shellcheck + run: | + set -euo pipefail + for attempt in {1..3}; do + if sudo apt-get update -qq \ + && sudo apt-get install -y --no-install-recommends bats shellcheck; then + exit 0 + fi + if [ "$attempt" -lt 3 ]; then + echo "Apt install failed, retrying in 5 seconds..." >&2 + sleep 5 + fi + done + echo "Apt install failed after 3 attempts" >&2 + exit 1 + + - name: Run bats suite + run: | + bats --print-output-on-failure tests/test_safety_checks.bats diff --git a/.markdownlint-cli2.yaml b/.markdownlint-cli2.yaml index 4f32107a4..90a511aeb 100644 --- a/.markdownlint-cli2.yaml +++ b/.markdownlint-cli2.yaml @@ -5,6 +5,10 @@ ignores: # levels, dangling cross-references, …) so scripts/agents-md-lint.sh has a # negative case per failure mode. Linting them as real docs would always fail. - "tests/fixtures/agents-md/**" + # Safety-checks fixtures mimic real PR descriptions, including the + # unfilled-template body that motivated epic #1200 — they start at ## by + # design (PR bodies have no H1 title), so MD041 would always fire. + - "tests/fixtures/safety-checks/**" config: MD010: diff --git a/scripts/lib/safety-checks.sh b/scripts/lib/safety-checks.sh new file mode 100644 index 000000000..e0225064a --- /dev/null +++ b/scripts/lib/safety-checks.sh @@ -0,0 +1,311 @@ +#!/usr/bin/env bash +# safety-checks.sh — sourceable PR-safety signal helpers. +# +# This library provides metadata-only signal functions that scripts can source +# to score or triage a pull request without executing any PR code. The first +# resident is sc_description_missing, the fix for the #1976 regression where +# an empty PR template was scored 0/5 missing — the template's own headings +# and HTML comments contain the keywords a naive grep looks for. +# +# Signal contract (shared by every sc_* helper in this file): +# stdin / args — PR metadata only (title, body, author_association, ...). +# stdout — a single integer or a `key=value` line, suitable for a scorer. +# stderr — human-readable warnings; a signal that cannot be read MUST +# fail closed (return non-zero) so the caller escalates to a +# human rather than scoring the PR "not spam". +# +# Related: epic combsmith/.github#1200 (spam-pr-guard), story #1202 (this +# function), story #1203 (the scorer that will consume these signals). + +# --------------------------------------------------------------------------- +# Description-quality signal +# --------------------------------------------------------------------------- +# The five canonical sections expected in a PR body. The order is irrelevant +# (each is matched independently); the labels are the keys the scorer reports. +# Kept as data (not a hardcoded regex) so a future config can change the +# section set without touching the matcher — the AGENTS.md "data not code" rule. +SC_DESCRIPTION_SECTION_KEYS=(problem risk test-plan rollback monitoring) + +# One case-insensitive ERE per key, matched against the heading line only. +# The keyword must START at a word boundary (so body text saying "problematic" +# does not drag the Problem section into "present"), but may extend with +# arbitrary trailing letters so plurals and variants in headings — "Risks and +# Mitigations", "Rollback Plan", "Monitoring and alerts" — still match. The +# trailing [[:alpha:]]* applies ONLY to heading matching; body-text classification +# never consults these patterns, so "problematic" in prose does not confuse it. +# The test-plan key additionally accepts the bare verb forms real PRs use — +# "Tests", "Testing", "How to test" — with the "plan" segment optional, so a +# "## Tests" heading is not mis-flagged as a missing test section. +SC_DESCRIPTION_SECTION_PATTERNS=( + '(^|[^[:alnum:]])problems?([^[:alnum:]]|$)' + '(^|[^[:alnum:]])risks?([^[:alnum:]]|$)' + '(^|[^[:alnum:]])test(s|ing)?([[:space:]-]?plan(s)?)?([^[:alnum:]]|$)' + '(^|[^[:alnum:]])rollback(s)?([^[:alnum:]]|$)' + '(^|[^[:alnum:]])monitoring([^[:alnum:]]|$)' +) + +# Lines whose entire trimmed-and-lowercased content matches one of these forms +# count as placeholder filler, not real body text. Keeping the list short and +# literal — generic heuristics (e.g. "any line that is just checkbox markers") +# would over-match genuine content. "_No response_" is GitHub's own default +# when an issue-form field is left empty, and the two spelling variants cover +# what the PR-form family actually emits. +SC_DESCRIPTION_PLACEHOLDER_FORMS=( + '_no response_' + 'no response' + '*no response*' +) + +# sc_description_missing — count how many of the five canonical sections lack +# real body text in a PR description, returning the integer on stdout. +# +# Contract: +# stdin — the raw PR body (markdown). +# stdout — a single integer in [0..5]; 5 means every section is empty. +# stderr — a human-readable warning when the body cannot be read. +# +# A section is "present" iff a markdown heading matches one of the five +# SC_DESCRIPTION_SECTION_PATTERNS AND at least one non-heading, non-comment, +# non-blank, non-structural line of real text sits under that heading before +# the next heading (or end-of-body). Lines inside fenced code blocks, HTML +# comments, Markdown thematic breaks (`---`, `***`, `___`), and known +# placeholder forms (`_No response_`) do NOT count — the #1976 regression +# class includes a draft that looks filled because every section carries a +# default placeholder and a horizontal rule between sections. +# +# Fail-closed: a hard read failure (cat exits non-zero) returns 1 and the +# caller must treat this as "escalate to human". An intentionally empty +# body is NOT a read failure; it is a valid 5/5 missing. +sc_description_missing() { + local body + if ! body="$(cat)"; then + echo "sc_description_missing: failed to read PR body from stdin" >&2 + return 1 + fi + + local -a found=(0 0 0 0 0) + local -a current=() # 0-based indexes of sections the last heading opened + local in_comment=0 # whether we are inside a span + local in_fence=0 # whether we are inside a ``` or ~~~ fenced block + local fence_char="" # the backtick/tilde that opened the current fence + local fence_len=0 # how many of fence_char the opening line held + local line stripped tail + + # Walk the body line by line. Each iteration either (a) continues/exits a + # multi-line HTML comment, (b) continues/exits a fenced code block, + # (c) classifies the line as a heading (opens a section), or (d) treats + # the line as body text under the current section (filtering out structural + # and placeholder lines). + while IFS= read -r line || [[ -n "$line" ]]; do + + # (a1) Inside a fenced code block — every line is code until the matching + # close. Fences dominate: HTML-comment markers inside code are code, and a + # `#` prefix inside code is not a heading. Same regression class as #1976 + # if we skip this (a `# problem` inside a ``` block would otherwise open + # the Problem section). + if (( in_fence )); then + _sc_is_fence_close "$line" "$fence_char" "$fence_len" && { + in_fence=0 + fence_char="" + fence_len=0 + } + continue + fi + + # (a2) Still inside a multi-line HTML comment: consume up to the closer. + if (( in_comment )); then + if [[ "$line" == *"-->"* ]]; then + tail="${line#*-->}" + in_comment=0 + # Fall through with `line` set to the post-comment tail so trailing + # text on the same line is still inspected. + line="$tail" + else + continue + fi + fi + + # Strip inline comments. If a comment opens but does not close on this + # line, we flip in_comment and keep whatever text precedes the opener. + stripped="" + while [[ -n "$line" ]]; do + if [[ "$line" == *""* ]]; then + line="${line#*-->}" + else + line="" + in_comment=1 + fi + else + stripped+="$line" + line="" + fi + done + + # (b) Fence open? Record the delimiter and skip. CommonMark allows an info + # string after the opening fence (e.g. ```` ```bash ````) — we don't care + # what it is; the whole line still just opens the fence. A code block IS + # real content, so credit every open section BEFORE skipping the fence + # body (otherwise a section whose only content is a code snippet would + # score as missing — exactly the opposite of what we want). + local fence_open_char + fence_open_char="$(_sc_fence_open_char "$stripped")" + if [[ -n "$fence_open_char" ]]; then + fence_char="$fence_open_char" + fence_len="$(_sc_fence_run_length "$stripped" "$fence_char")" + in_fence=1 + if (( ${#current[@]} > 0 )); then + local idx + for idx in "${current[@]}"; do + found[idx]=1 + done + fi + continue + fi + + # (c) Heading line? Record EVERY yet-unmet section key it matches. A single + # heading can open more than one section ("## Risk and Rollback", "## Problem + # / Risk"), so we collect all matching indexes rather than stopping at the + # first — each section is matched independently per the contract above. + # ATX headings per CommonMark: at most three leading SPACES (a leading tab is + # four-column indentation, i.e. an indented code block), then one-to-six `#` + # (seven or more is not a heading) followed by whitespace or end-of-line. So + # a line like ` # problem` or `\t## problem` must NOT register as a heading. + local heading_regex='^ {0,3}#{1,6}([[:space:]]|$)' + if [[ "$stripped" =~ $heading_regex ]]; then + current=() + # Native lowercase (bash 4+) — avoids a per-line subshell + tr fork. + local lower="${stripped,,}" + local i + for i in "${!SC_DESCRIPTION_SECTION_PATTERNS[@]}"; do + if (( found[i] )); then continue; fi + if [[ "$lower" =~ ${SC_DESCRIPTION_SECTION_PATTERNS[i]} ]]; then + current+=("$i") + fi + done + continue + fi + + # (d) Body text. If it sits under one or more open sections AND has at + # least one non-whitespace character AND is neither a Markdown thematic + # break nor a known placeholder form, mark every one of those sections + # found. The thematic-break / placeholder filters are what keep a draft + # that reads as filled-but-empty (every section a `_No response_` under a + # `---` divider) from scoring 0/5 missing. + if (( ${#current[@]} > 0 )); then + local non_space_regex='[^[:space:]]' + if [[ "$stripped" =~ $non_space_regex ]] \ + && ! _sc_is_thematic_break "$stripped" \ + && ! _sc_is_placeholder "$stripped"; then + local idx + for idx in "${current[@]}"; do + found[idx]=1 + done + fi + fi + done <<<"$body" + + local miss=0 j + for j in "${!SC_DESCRIPTION_SECTION_KEYS[@]}"; do + if (( ! found[j] )); then miss=$((miss + 1)); fi + done + printf '%d\n' "$miss" +} + +# --------------------------------------------------------------------------- +# Private helpers (prefix _sc_ marks them off the public sc_* surface) +# --------------------------------------------------------------------------- + +# _sc_fence_open_char — return the fence delimiter character (` or ~) if the +# argument is a CommonMark fenced-code-block OPEN line, else empty. CommonMark +# requires three or more of the same delimiter at columns 0..3; anything may +# follow as an info string. +_sc_fence_open_char() { + local s="$1" + local backtick_open_regex='^ {0,3}`{3,}' + local tilde_open_regex='^ {0,3}~{3,}' + if [[ "$s" =~ $backtick_open_regex ]]; then + printf '`' + return 0 + fi + if [[ "$s" =~ $tilde_open_regex ]]; then + printf '~' + return 0 + fi +} + +# _sc_fence_run_length — count the opening fence delimiter run on a line that +# already matched _sc_fence_open_char. We walk past the leading whitespace and +# count the delimiter run; this is the minimum length the closing fence must +# match or exceed per CommonMark. +_sc_fence_run_length() { + local s="$1" ch="$2" + local trimmed="${s#"${s%%[![:space:]]*}"}" + local n=0 + while [[ "${trimmed:$n:1}" == "$ch" ]]; do + n=$((n + 1)) + done + printf '%d' "$n" +} + +# _sc_is_fence_close — return true iff the line is a valid closing fence for +# the currently-open fence. CommonMark: at most three leading spaces, then at +# least fence_len copies of fence_char, optional trailing whitespace, nothing +# else. +_sc_is_fence_close() { + local s="$1" ch="$2" needed="$3" + local trimmed="${s#"${s%%[![:space:]]*}"}" + # Strip trailing whitespace. + trimmed="${trimmed%"${trimmed##*[![:space:]]}"}" + # The trimmed line must be ch repeated at least `needed` times, nothing else. + (( ${#trimmed} >= needed )) || return 1 + local i + for (( i = 0; i < ${#trimmed}; i++ )); do + [[ "${trimmed:i:1}" == "$ch" ]] || return 1 + done + return 0 +} + +# _sc_is_thematic_break — true iff the line is a CommonMark thematic break: +# at most three leading spaces, then three or more of `-`, `*`, or `_` +# (all the same character), optionally separated by any amount of whitespace, +# and nothing else on the line. +_sc_is_thematic_break() { + local s="$1" + local trimmed="${s#"${s%%[![:space:]]*}"}" + trimmed="${trimmed%"${trimmed##*[![:space:]]}"}" + (( ${#trimmed} >= 3 )) || return 1 + local first="${trimmed:0:1}" + case "$first" in + -|\*|_) ;; + *) return 1 ;; + esac + local count=0 ch i + for (( i = 0; i < ${#trimmed}; i++ )); do + ch="${trimmed:i:1}" + if [[ "$ch" == "$first" ]]; then + count=$((count + 1)) + elif [[ "$ch" != " " && "$ch" != $'\t' ]]; then + return 1 + fi + done + (( count >= 3 )) +} + +# _sc_is_placeholder — true iff the stripped line, trimmed and lowercased, +# matches any of SC_DESCRIPTION_PLACEHOLDER_FORMS. These are the GitHub +# issue-form defaults that look like body text but mean "contributor skipped +# this field". +_sc_is_placeholder() { + local s="$1" + local trimmed="${s#"${s%%[![:space:]]*}"}" + trimmed="${trimmed%"${trimmed##*[![:space:]]}"}" + local lower="${trimmed,,}" + local form + for form in "${SC_DESCRIPTION_PLACEHOLDER_FORMS[@]}"; do + [[ "$lower" == "$form" ]] && return 0 + done + return 1 +} diff --git a/tests/fixtures/safety-checks/filled-description.md b/tests/fixtures/safety-checks/filled-description.md new file mode 100644 index 000000000..57e52f964 --- /dev/null +++ b/tests/fixtures/safety-checks/filled-description.md @@ -0,0 +1,25 @@ +## Problem + +The compliance audit script crashed on repos without a default branch set. +Repro: `scripts/compliance-audit.sh --repo combsmith/empty-fixture`. + +## Risk + +Low. The fix is a two-line guard around a `jq` expression that was dereferencing +a null default_branch; no schema change, no state migration. + +## Test Plan + +Added a bats fixture covering the null-default-branch case to +`tests/test_safety_checks.bats`; existing cases stay green. Ran the full suite +locally. + +## Rollback + +Revert this commit; the pre-change path was identical except for the guard. +No cleanup needed. + +## Monitoring + +The daily `compliance-audit-and-improvement` workflow writes a run summary +that will light up on the next cron tick if the guard regresses. diff --git a/tests/fixtures/safety-checks/partial-two-of-five.md b/tests/fixtures/safety-checks/partial-two-of-five.md new file mode 100644 index 000000000..d2beaeed5 --- /dev/null +++ b/tests/fixtures/safety-checks/partial-two-of-five.md @@ -0,0 +1,17 @@ +## Problem + +The compliance script dies on repos without a default branch. + +## Risk + + + +## Test Plan + +Added a bats fixture; existing suite stays green. + +## Rollback + + + +## Monitoring diff --git a/tests/fixtures/safety-checks/pr-1976-unfilled-template.md b/tests/fixtures/safety-checks/pr-1976-unfilled-template.md new file mode 100644 index 000000000..d4ffb157a --- /dev/null +++ b/tests/fixtures/safety-checks/pr-1976-unfilled-template.md @@ -0,0 +1,28 @@ + + +## Problem + + + +## Risk + + + +## Test Plan + + + +## Rollback + + + +## Monitoring + + diff --git a/tests/test_safety_checks.bats b/tests/test_safety_checks.bats new file mode 100644 index 000000000..ac1ca8dba --- /dev/null +++ b/tests/test_safety_checks.bats @@ -0,0 +1,224 @@ +#!/usr/bin/env bats +# Unit tests for scripts/lib/safety-checks.sh — the sourceable PR-safety +# signal helpers that back the Layer 1 spam-pr-guard scorer (epic #1200, +# story #1202). +# +# The function under test, sc_description_missing, replaces the naive +# keyword grep in the prior PR-safety path that scored an UNFILLED PR +# template as 0/5 missing. The template's own headings and HTML comments +# contain all five keywords, so the raw-body matcher reported "complete" +# on an empty template — the exact regression that let +# combsmith/.github-private#1976 ship through automation. +# +# These cases cover: +# - the #1976 fixture reports a near-max missing count (regression anchor); +# - a genuinely filled description still reports 0 missing (counterfactual); +# - an intermediate 2-of-5 case reports 3 (ordering + arithmetic); +# - an empty body reports 5; +# - the matcher correctly ignores HTML comment bodies, heading-text keyword +# echoes, and keyword-as-prefix (e.g. "problematic") false positives. +# +# Pattern follows tests/agents_md_lint.bats: sourceable pure helper + pure +# bats cases over fixtures under tests/fixtures/safety-checks/. + +bats_require_minimum_version 1.5.0 + +setup() { + REPO_ROOT="$(cd "$(dirname "$BATS_TEST_FILENAME")/.." && pwd)" + LIB="$REPO_ROOT/scripts/lib/safety-checks.sh" + FIX="$REPO_ROOT/tests/fixtures/safety-checks" + # shellcheck source=/dev/null + source "$LIB" +} + +# --- fixture-level cases -------------------------------------------------- + +@test "sc_description_missing: the #1976 unfilled-template fixture reports a near-max missing count" { + # The regression anchor. Pre-fix behaviour was 0 (false 'complete'); after + # the fix every section is empty-after-stripping, so all five are missing. + run bash -c "source '$LIB' && sc_description_missing < '$FIX/pr-1976-unfilled-template.md'" + [ "$status" -eq 0 ] + [ "$output" = "5" ] +} + +@test "sc_description_missing: a genuinely filled description still reports 0 missing (counterfactual)" { + run bash -c "source '$LIB' && sc_description_missing < '$FIX/filled-description.md'" + [ "$status" -eq 0 ] + [ "$output" = "0" ] +} + +@test "sc_description_missing: 2 of 5 sections filled reports 3 missing" { + run bash -c "source '$LIB' && sc_description_missing < '$FIX/partial-two-of-five.md'" + [ "$status" -eq 0 ] + [ "$output" = "3" ] +} + +# --- inline, construct-level cases --------------------------------------- + +@test "sc_description_missing: empty body reports 5 missing" { + run bash -c "source '$LIB' && printf '' | sc_description_missing" + [ "$status" -eq 0 ] + [ "$output" = "5" ] +} + +@test "sc_description_missing: a heading alone with no body text does NOT count as present" { + # The core #1976 fix: the heading line itself must not satisfy its own section. + body=$'## Problem\n\n## Risk\n\n## Test Plan\n\n## Rollback\n\n## Monitoring\n' + run bash -c "source '$LIB'; printf '%s' \"\$1\" | sc_description_missing" _ "$body" + [ "$status" -eq 0 ] + [ "$output" = "5" ] +} + +@test "sc_description_missing: an HTML comment under a heading does NOT count as present" { + # Comments carry the template's helper text and must be stripped before the + # body-text check — otherwise the regression returns. + body=$'## Problem\n\n\n\n## Risk\n\n## Test Plan\n\n## Rollback\n\n## Monitoring\n' + run bash -c "source '$LIB'; printf '%s' \"\$1\" | sc_description_missing" _ "$body" + [ "$status" -eq 0 ] + [ "$output" = "5" ] +} + +@test "sc_description_missing: multi-line HTML comment is stripped across newlines" { + body=$'## Problem\n\n\n\n## Risk\n## Test Plan\n## Rollback\n## Monitoring\n' + run bash -c "source '$LIB'; printf '%s' \"\$1\" | sc_description_missing" _ "$body" + [ "$status" -eq 0 ] + [ "$output" = "5" ] +} + +@test "sc_description_missing: an H1 whose title matches a section keyword, with body text, counts as present" { + # ATX H1 headings open sections just like H2/H3 (one-to-six hashes per + # CommonMark). 'Design Problem Statement' is an H1 containing the whole word + # 'problem', so the prose beneath it marks the Problem section present. + body=$'# Design Problem Statement\n\nSome prose about scope.\n\n## Risk\n\nreal content\n\n## Test Plan\n\nreal content\n\n## Rollback\n\nreal content\n\n## Monitoring\n\nreal content\n' + run bash -c "source '$LIB'; printf '%s' \"\$1\" | sc_description_missing" _ "$body" + [ "$status" -eq 0 ] + [ "$output" = "0" ] +} + +@test "sc_description_missing: 'problematic' prose does NOT satisfy the Problem section via prefix match" { + # Whole-word boundary — 'problematic' must not match 'problem'. + body=$'## Other\n\nThis is problematic behaviour we want to avoid.\n\n## Risk\n\nx\n\n## Test Plan\n\nx\n\n## Rollback\n\nx\n\n## Monitoring\n\nx\n' + run bash -c "source '$LIB'; printf '%s' \"\$1\" | sc_description_missing" _ "$body" + [ "$status" -eq 0 ] + # Problem never opens because no heading contains the whole word 'problem'. + [ "$output" = "1" ] +} + +@test "sc_description_missing: an indented code block containing '# problem' does NOT register as a heading" { + # CommonMark: four or more leading spaces turn a line into an indented code + # block, not an ATX heading. The Problem section should still be missing. + body=$' # problem (inside a code block)\n\n## Risk\n\nx\n\n## Test Plan\n\nx\n\n## Rollback\n\nx\n\n## Monitoring\n\nx\n' + run bash -c "source '$LIB'; printf '%s' \"\$1\" | sc_description_missing" _ "$body" + [ "$status" -eq 0 ] + [ "$output" = "1" ] +} + +@test "sc_description_missing: a tab-indented '## problem' does NOT register as a heading" { + # CommonMark treats a leading tab as four-column indentation (an indented + # code block), not ATX heading indentation, so only literal spaces count. + # The Problem section must therefore stay missing. + body=$'\t## Problem\n\nx\n\n## Risk\n\nx\n\n## Test Plan\n\nx\n\n## Rollback\n\nx\n\n## Monitoring\n\nx\n' + run bash -c "source '$LIB'; printf '%s' \"\$1\" | sc_description_missing" _ "$body" + [ "$status" -eq 0 ] + [ "$output" = "1" ] +} + +@test "sc_description_missing: a run of seven '#' does NOT register as a heading" { + # ATX headings are limited to six hashes; seven or more is plain text, so the + # Problem section stays missing. + body=$'####### Problem\n\nx\n\n## Risk\n\nx\n\n## Test Plan\n\nx\n\n## Rollback\n\nx\n\n## Monitoring\n\nx\n' + run bash -c "source '$LIB'; printf '%s' \"\$1\" | sc_description_missing" _ "$body" + [ "$status" -eq 0 ] + [ "$output" = "1" ] +} + +@test "sc_description_missing: a combined heading opens every section it names" { + # '## Risk and Rollback' matches both the risk and rollback keys; the body + # beneath must mark BOTH present, leaving only problem/test-plan/monitoring + # missing (3), not 4. + body=$'## Risk and Rollback\n\nreal content\n' + run bash -c "source '$LIB'; printf '%s' \"\$1\" | sc_description_missing" _ "$body" + [ "$status" -eq 0 ] + [ "$output" = "3" ] +} + +@test "sc_description_missing: a bare '## Tests' heading satisfies the test-plan section" { + # Real PRs use plain verb forms ('Tests', 'Testing') for the test section; + # the test-plan key accepts them with the 'plan' segment optional. + body=$'## Problem\n\nx\n\n## Risk\n\nx\n\n## Tests\n\nx\n\n## Rollback\n\nx\n\n## Monitoring\n\nx\n' + run bash -c "source '$LIB'; printf '%s' \"\$1\" | sc_description_missing" _ "$body" + [ "$status" -eq 0 ] + [ "$output" = "0" ] +} + +@test "sc_description_missing: heading variants (Rollback Plan, Test plan / verification) still match" { + body=$'## Problem\n\nx\n\n## Risks and Mitigations\n\nx\n\n## Test plan / verification\n\nx\n\n## Rollback Plan\n\nx\n\n## Monitoring and alerts\n\nx\n' + run bash -c "source '$LIB'; printf '%s' \"\$1\" | sc_description_missing" _ "$body" + [ "$status" -eq 0 ] + [ "$output" = "0" ] +} + +# --- non-content filters (fenced code, thematic break, placeholder forms) --- + +@test "sc_description_missing: a '# problem' inside a fenced code block does NOT register as a heading" { + # Same regression class as #1976 — a code snippet in the Test Plan section + # must not open the Problem section via its own code comment. The Problem + # section is empty beyond its heading and must still count as missing. + body=$'## Problem\n\n## Risk\n\nx\n\n## Test Plan\n\n```shell\n# problem: this is a code example, not a heading\necho hi\n```\n\n## Rollback\n\nx\n\n## Monitoring\n\nx\n' + run bash -c "source '$LIB'; printf '%s' \"\$1\" | sc_description_missing" _ "$body" + [ "$status" -eq 0 ] + [ "$output" = "1" ] +} + +@test "sc_description_missing: a tilde-fenced code block is also tracked" { + body=$'## Problem\n\n## Risk\n\nx\n\n## Test Plan\n\n~~~\n# problem: tilde-fenced example\n~~~\n\n## Rollback\n\nx\n\n## Monitoring\n\nx\n' + run bash -c "source '$LIB'; printf '%s' \"\$1\" | sc_description_missing" _ "$body" + [ "$status" -eq 0 ] + [ "$output" = "1" ] +} + +@test "sc_description_missing: a Markdown thematic break under an otherwise empty section does NOT count as body text" { + # '---' / '***' / '___' are structural rules, not content. The Problem + # section only carries a horizontal rule and must still count as missing. + body=$'## Problem\n\n---\n\n## Risk\n\nx\n\n## Test Plan\n\nx\n\n## Rollback\n\nx\n\n## Monitoring\n\nx\n' + run bash -c "source '$LIB'; printf '%s' \"\$1\" | sc_description_missing" _ "$body" + [ "$status" -eq 0 ] + [ "$output" = "1" ] +} + +@test "sc_description_missing: '_No response_' placeholder lines do NOT count as body text" { + # GitHub form default: when a contributor leaves an issue-form field empty, + # the field renders as '_No response_'. Treating it as real content turns a + # fully-skipped template into 0/5 missing. Three forms covered. + body=$'## Problem\n\n_No response_\n\n## Risk\n\n*No response*\n\n## Test Plan\n\nNo response\n\n## Rollback\n\nx\n\n## Monitoring\n\nx\n' + run bash -c "source '$LIB'; printf '%s' \"\$1\" | sc_description_missing" _ "$body" + [ "$status" -eq 0 ] + [ "$output" = "3" ] +} + +# --- fail-closed contract (stdin read failure) ---------------------------- + +@test "sc_description_missing: a failing stdin reader returns non-zero and emits no numeric score" { + # Fail-closed contract: if the body cannot be read, the function must exit + # non-zero so the caller (Phase 2's scorer) escalates to a human rather than + # scoring the PR 'not spam'. We simulate a hard read failure by shadowing + # `cat` with a shell function that always exits 1 — the shadow is inherited + # by the subshell that `$(cat)` spawns inside the function. + run bash -c "cat() { return 1; }; export -f cat; source '$LIB' && sc_description_missing" + # Expected: non-zero exit, no numeric stdout. (The function's own stderr + # message is captured in BATS stderr, not stdout, so stdout stays clean.) + [ "$status" -ne 0 ] + # output must not be a plain digit string a caller could mistake for a + # valid 0..5 score. + [[ ! "$output" =~ ^[0-9]+$ ]] +} + +# --- shellcheck gate ------------------------------------------------------ + +@test "scripts/lib/safety-checks.sh passes shellcheck" { + if ! command -v shellcheck >/dev/null 2>&1; then + skip "shellcheck not installed" + fi + run shellcheck --shell=bash "$LIB" + [ "$status" -eq 0 ] +}