fix(outside-review): read a verdict line's inline code and reject caveated approvals (#3925 round-2 floor findings)

Co-authored-by: capy-ai[bot] <230910855+capy-ai[bot]@users.noreply.github.com>

docs/troubleshooting.md

@@ -184,9 +184,10 @@ the readiness dashboard and the PR body, and never count it as a pass.
   heading with the value on the next line. A last verdict that is not an
   approval (`ready`, `LGTM`, `approve`, `ship it`, `pass`, `clean`, `good to
   go`), such as `Can't verify`, `blocked`, `revise` or an empty one, or an
-  approval that says something was excluded, blocked, skipped, not run or
-  could not be verified, is [`unverified`](#outside-review-unverified-verdict),
-  never `clean`.
+  approval that says something was excluded, blocked, skipped, not run, not
+  executed or could not be verified (or "not yet", or a question such as
+  `Ready? No`), is [`unverified`](#outside-review-unverified-verdict), never
+  `clean`. Inline code inside a verdict line is read as text.
 - **Numbered findings.** In a reply with a verdict line, every numbered item
   at the start of a line (`1. x`, `1. **Nonblocking — x**`) is a finding,
   unless the reply concludes that it found nothing. When one has no readable

lib/outside-review-result.ts

@@ -202,8 +202,8 @@ function plainReview(text: string): string {
  * an opener closes only when a later marker of its kind is at least as long.
  */
 const CODE_LABEL_BEFORE = /^[\t ]*(?:>[\t ]*)?(?:#{1,6}[\t ]+|[-+*][\t ]+|\(?\d{1,3}[.)][\t ]+)?(?:\*\*|__)?$|\b(?:severity|priority)[\t ]*[:=][\t ]*(?:\*\*|__)?$|\|[\t ]*(?:\*\*)?$/i;
-/** A verdict label before an inline span: `Verdict: \`ready\`` keeps its value. */
-const VERDICT_BEFORE = /^[\t ]*(?:>[\t ]*)*(?:#{1,6}[\t ]+|[-+*][\t ]+)?(?:\*\*|__)?(?:(?:overall|final)[\t ]+)?verdict(?:\*\*|__)?[\t ]*(?::|\u2014|\u2013|-)?[\t ]*(?:\*\*|__)?[\t ]*$/i;
+/** A verdict line keeps every inline span: `Verdict: \`ready\` (tests \`not run\`)` is read whole. */
+const VERDICT_LINE = /^[\t ]*(?:>[\t ]*)*(?:#{1,6}[\t ]+|[-+*][\t ]+)?(?:\*\*|__)?(?:(?:overall|final)[\t ]+)?verdict\b/i;
 function withoutCode(text: string, readUnclosed = false): string {
   const lines = text.split(/\r?\n/);
   const markers = lines.map(line => /^[\t ]*(`{3,}|~{3,})/.exec(line)?.[1]);
@@ -227,7 +227,7 @@ function withoutCode(text: string, readUnclosed = false): string {
     if (marker) { if (closes[i]) fence = marker; return ''; }
     return line.replace(/`([^`]+)`/g, (span, inner: string, at: number) =>
       inner.trim() === 'NO_FINDINGS' || /^\[?(?:P[0-3]|critical|high|medium|low)\]?$/i.test(inner.trim()) && CODE_LABEL_BEFORE.test(line.slice(0, at))
-        || VERDICT_BEFORE.test(line.slice(0, at)) ? inner : ' ');
+        || VERDICT_LINE.test(line) ? inner : ' ');
   }).join('\n');
 }
 
@@ -269,9 +269,9 @@ function lastVerdict(plain: string): string | undefined {
   return undefined;
 }
 /** A verdict that approves the change. */
-const READY_VERDICT = /^(?:ready|lgtm|approved?|ship(?:[\t ]+it)?|pass(?:ed)?|clean|good to (?:go|merge|ship))\b/i;
+const READY_VERDICT = /^(?:ready|lgtm|approved?|ship(?:[\t ]+it)?|pass(?:ed)?|clean|good to (?:go|merge|ship))\b(?![-?/])/i;
 /** A verdict that says the reviewer could not check, run or see something is never a ready one (#3925). */
-const UNVERIFIED_CLAIM = /\b(?:can['\u2019]?t|cannot|can not|couldn['\u2019]t|could not|unable to|not able to|did not|didn['\u2019]t)\s+(?:be\s+)?(?:verif|run|test|check|open|read|review)|\bunverifi|\bnot (?:been )?(?:verified|run|tested|checked|reviewed)\b|\b(?:excluded|blocked|skipped)\b/i;
+const UNVERIFIED_CLAIM = /\b(?:can['\u2019]?t|cannot|can not|couldn['\u2019]t|could not|unable to|not able to|did not|didn['\u2019]t)\s+(?:be\s+)?(?:verif|run|test|check|open|read|review|execut)|\bunverifi|\bnot (?:yet|until)\b|\bnot (?:been )?(?:verified|run|tested|checked|reviewed|executed)\b|\b(?:excluded|blocked|skipped)\b/i;
 /** A numbered item at the start of a line; in a reply with a `Verdict:` line it is a finding (#3925). */
 const NUMBERED_ITEM = /^(?:\*\*|__)?\(?\d{1,3}[.)][\t ]+\S.*$/gm;
 

test/outside-review-unparsed-severity.test.ts

@@ -226,6 +226,8 @@ describe('#3925 an unrecognized verdict is not clean; numbered findings never pr
         'Verdict: ready, but I could not verify the migration', 'Verdict: ready (unverified)',
         "> Verdict: Can't verify (suites blocked)", "## Verdict\nCan't verify (one changed file excluded)", "Overall verdict: can't verify",
         "Verdict \u2014 can't verify", 'Verdict: ready, though one changed file was excluded', 'Verdict: ready (tests not run, sandbox blocked)',
+        'Verdict: `ready` (tests `not run`)', 'Verdict: Ready? Not until the migration is fixed.', 'Verdict: clean-up required before merge',
+        'Verdict: ready, but I did not execute the suites', 'Verdict: ready (tests not executed)',
       ])
         expect({ gate, verdictLine, ...see(`No issues found.\n${verdictLine}`, gate) }).toEqual({ gate, verdictLine, verdict: 'unverified', reason: 'unverified_verdict', unlabeled: 0 });
   });