diff --git a/.github/workflows/scripts/strip-non-invoking-markup.sh b/.github/workflows/scripts/strip-non-invoking-markup.sh index 65f2529..099942b 100755 --- a/.github/workflows/scripts/strip-non-invoking-markup.sh +++ b/.github/workflows/scripts/strip-non-invoking-markup.sh @@ -70,6 +70,51 @@ function run_len(s, ch, n) { return n } +# Whether `bare` is a CommonMark thematic break: three or more of a single one +# of `- * _`, with only spaces or tabs between. Written as a character count +# rather than a regex because the POSIX ERE that awk uses has no backreference, +# so a same-character-repeated pattern (a `\1` in PCRE) silently never matches. +function is_thematic_break(bare, ch, i, c, n) { + ch = substr(bare, 1, 1) + if (ch != "-" && ch != "*" && ch != "_") return 0 + n = 0 + for (i = 1; i <= length(bare); i++) { + c = substr(bare, i, 1) + if (c == ch) n++ + else if (c != " " && c != "\t") return 0 + } + return (n >= 3) +} + +# Whether `bare` is a setext heading underline: a run of only `=` or only `-`, +# trailing spaces/tabs allowed. This is a *heading* underline only when the line +# before it is paragraph text, which the caller tracks separately -- the same +# characters are otherwise a thematic break (`- * _`, >=3), a list marker, or +# ordinary text. CommonMark makes the two underline characters symmetric, and an +# indented code block opens after either with no blank line, so the `-` case +# that `is_thematic_break` already covers for >=3 dashes needs its `=` twin here +# (plus 1-2 dash underlines, which are not thematic breaks). +function is_setext_underline(bare) { + return bare ~ /^=+[ \t]*$/ || bare ~ /^-+[ \t]*$/ +} + +# Whether this line is a leaf block -- an ATX heading or a thematic break -- +# after which an indented code block may begin with no blank line. `bare` is the +# line with its indentation removed, `indent` its width. These two shapes need +# no context; a setext underline also qualifies but only after paragraph text, +# so the caller handles it. A list item deliberately does NOT qualify: an +# indented line after one is a list continuation, not code, and stripping it is +# the over-stripping error the mention gate must avoid (the list-continuation +# cases added in #345 guard exactly this). Both shapes are only valid at three +# columns of indentation or fewer; deeper, they are themselves code. +function heading_or_break(bare, indent) { + if (indent > 3) return 0 + # ATX heading: 1-6 `#` then a space/tab or end of line. + if (bare ~ /^#{1,6}([ \t]|$)/) return 1 + if (is_thematic_break(bare)) return 1 + return 0 +} + # Replace every closed inline code span in `text` with the placeholder. # # This runs once over the WHOLE remaining body rather than per line, because a @@ -121,7 +166,8 @@ function strip_spans(text, out, i, n, run, j, closerun, found) { BEGIN { in_fence = 0; fence_char = ""; fence_len = 0 in_icode = 0 - prev_blank = 1 # start of input behaves like a blank line + prev_opens_icode = 1 # start of input can open an indented code block + prev_para = 0 # ... but is not itself paragraph text (no setext under it) kept = ""; nkept = 0 } @@ -142,23 +188,26 @@ BEGIN { substr(bare, run_len(bare, fence_char) + 1) ~ /^[ \t]*$/) { in_fence = 0 } - prev_blank = blank + prev_opens_icode = 0; prev_para = 0 next } # --- indented code block ---------------------------------------------- - # Four columns of indentation after a blank line opens one, and it runs - # until a non-blank line dedents to three columns or fewer. The blank-line - # precondition is what keeps an indented *list continuation* out of this - # branch: those follow their list item directly, so over-stripping them - # would drop a genuine request, which is the expensive error for the - # mention gate sharing this script. + # Four columns of indentation opens one when the preceding line does not leave + # an open paragraph or list for it to lazily continue. `prev_opens_icode` + # carries that from the previous iteration: it is set for a blank line, an ATX + # heading, a thematic break (`heading_or_break`), or a setext underline sitting + # under paragraph text. It runs until a non-blank line dedents to three columns + # or fewer. The precondition is what keeps an indented *list continuation* out + # of this branch: those follow their list item directly, so over-stripping them + # would drop a genuine request, which is the expensive error for the mention + # gate sharing this script. if (in_icode) { - if (blank || indent >= 4) { prev_blank = blank; next } + if (blank || indent >= 4) { prev_opens_icode = 0; prev_para = 0; next } in_icode = 0 - } else if (prev_blank && !blank && indent >= 4) { + } else if (prev_opens_icode && !blank && indent >= 4) { in_icode = 1 - prev_blank = blank + prev_opens_icode = 0; prev_para = 0 next } @@ -167,17 +216,30 @@ BEGIN { fence_char = lead fence_len = run_len(bare, lead) in_fence = 1 - prev_blank = blank + prev_opens_icode = 0; prev_para = 0 next } - if (lead == ">") { prev_blank = blank; next } + if (lead == ">") { prev_opens_icode = 0; prev_para = 0; next } } # Kept lines are buffered rather than printed, so the span scan below can # see across line boundaries. kept = (nkept == 0) ? line : kept "\n" line nkept++ - prev_blank = blank + # Decide what the NEXT line inherits. A blank line, an ATX heading, or a + # thematic break (`hd`) opens an indented code block after it with no context; + # a setext underline does too, but only when it is actually *consumed* as one + # -- a `=`/`-` run (`ul`) sitting under paragraph text (`prev_para`). A ul-shaped + # run with no paragraph above it is not consumed: CommonMark makes it an + # ordinary new paragraph, so `prev_para` must go to 1 for the next line (a later + # underline could underline it), which is why `prev_para` keys on `consumed` + # rather than the raw `ul` match. Otherwise a chain of underline-shaped lines + # (`===` / `===` / indented request) mistracks state and under-strips. + hd = heading_or_break(bare, indent) + ul = (indent <= 3 && is_setext_underline(bare)) + consumed = (ul && prev_para) + prev_opens_icode = (blank || hd || consumed) + prev_para = (!blank && !hd && !consumed) } END { diff --git a/.github/workflows/scripts/tests/run-strip-non-invoking-markup-tests.sh b/.github/workflows/scripts/tests/run-strip-non-invoking-markup-tests.sh index ab4b037..22a5966 100755 --- a/.github/workflows/scripts/tests/run-strip-non-invoking-markup-tests.sh +++ b/.github/workflows/scripts/tests/run-strip-non-invoking-markup-tests.sh @@ -167,6 +167,61 @@ check "indentation without a preceding blank line is not code" \ $'some prose\n @claude review' \ $'some prose\n @claude review' +# An indented code block also opens after a thematic break or an ATX heading +# with no blank line: neither leaves an open paragraph for the indented line to +# lazily continue (CommonMark 0.31.2, "Indented code blocks"). A request quoted +# this way must still be stripped (gha#356). +check "indented code block opens after a thematic break" \ + $'---\n @claude review' \ + '---' + +check "indented code block opens after an ATX heading" \ + $'## Accepted phrasing\n @claude review' \ + '## Accepted phrasing' + +check "indented code block opens after a spaced thematic break" \ + $'* * *\n @claude review' \ + '* * *' + +# A list item is NOT such a predecessor: an indented line after it is a list +# continuation, not code, so it must survive -- and a lone-dash item must not +# be mistaken for a thematic break, which would over-strip a genuine request. +check "indented line after a list item is not code" \ + $'- a point\n @claude review' \ + $'- a point\n @claude review' + +# A setext heading underline (a run of only `=` or only `-` under paragraph text) +# is also such a predecessor. CommonMark makes the two underline characters +# symmetric; the `-` case is caught by the thematic-break check only for >=3 +# dashes, so the `=` twin and the 1-2 dash underlines are handled here. +check "indented code block opens after an = setext underline" \ + $'Heading\n======\n @claude review' \ + $'Heading\n======' + +check "indented code block opens after a single-dash setext underline" \ + $'Heading\n-\n @claude review' \ + $'Heading\n-' + +# But a run of `=` NOT under paragraph text is an ordinary paragraph, not a +# heading underline, so an indented line after it is a lazy continuation, not +# code -- stripping it would drop a genuine request. +check "= run at start of input is a paragraph, not a setext underline" \ + $'======\n @claude review' \ + $'======\n @claude review' + +check "= run after a blank line is a paragraph, not a setext underline" \ + $'intro\n\n======\n @claude review' \ + $'intro\n\n======\n @claude review' + +# A chain of underline-shaped lines: the first `===` has no paragraph above it so +# it is an ordinary paragraph, and the second `===` then underlines it into a +# heading -- after which the indented request is code and must be stripped. This +# only works if paragraph state tracks whether a line was actually *consumed* as +# an underline, not the raw match. +check "chain of two = runs forms a heading, stripping the indented request" \ + $'===\n===\n @claude review' \ + $'===\n===' + check "CRLF endings are normalized" \ $'@claude review\r\nthanks\r' \ $'@claude review\nthanks' diff --git a/changelog.d/strip-non-invoking-markup-block-predecessors.fixed.md b/changelog.d/strip-non-invoking-markup-block-predecessors.fixed.md new file mode 100644 index 0000000..102e21c --- /dev/null +++ b/changelog.d/strip-non-invoking-markup-block-predecessors.fixed.md @@ -0,0 +1,14 @@ +- **The `@claude review` matcher no longer fires on a mention quoted as an + indented code block that opens after a heading or a thematic break** + (gha#356). + `strip-non-invoking-markup.sh` only opened an indented code block after a + blank line, but CommonMark bars an indented block only from *interrupting a + paragraph* -- after a thematic break, an ATX heading, or a setext-heading + underline it opens with no blank line at all. + So a request quoted under a `---` or a `#`/`===` heading rendered as code on + GitHub yet slipped through the stripper and dispatched a review off quoted + text -- the same class of false dispatch the code-span and fence handling + already close. + A list item is deliberately still not treated as such a predecessor: an + indented line after one is a list continuation, not code, so stripping it + would drop a genuine request in the mention gate that shares this script.