fix: комментарий засчитывается вердиктом только по документу своей задачи

User-Visible: no
Issue: #454

.github/workflows/process.yml

@@ -97,9 +97,11 @@ jobs:
           #
           # Вердикты считаются ТОЛЬКО своего этапа: иначе вердикт по ТЗ съедал
           # цикл из бюджета код-ревью (#89 получило r2/4). Этап опознаётся по
-          # имени документа в теле комментария; документа нет — вердикт не
-          # посчитается. Недосчёт считался обратимой ошибкой — «даёт лишний
-          # заход» — и в этой оценке была ошибка, см. ниже.
+          # имени документа — раньше по подстроке маркера в теле комментария,
+          # теперь по имени документа ЭТОЙ задачи, `<MARKER>-<NUM>`: голая
+          # подстрока протекала на прозе. #454 поймала это на себе — разбор
+          # чужих задач в комментарии содержал `CODE-REVIEW`, и первый же
+          # код-ревью получил заход r3.
           #
           # Счёт по комментариям остаётся ровно тем же, но он БОЛЬШЕ НЕ
           # ЕДИНСТВЕННЫЙ (#454). Маркер этапа попадает в тело комментария,
@@ -116,14 +118,8 @@ jobs:
           # обоих, перерасчёт невозможен по построению.
           attempt=1; spent=0; spent_list=""
           if [ -n "$stage" ]; then
-            comments=$(gh issue view "$NUM" --repo "$REPO" --json comments)
-            of_stage="[.comments[] | select(.body | test(\"Вердикт:\")) | select(.body | test(\"$marker\"))]"
-            # Блокирующим считается вердикт, у которого в строке вердикта стоит
-            # «жёлтый» или «красный». Регистр и окружение слова не важны.
-            blocking="$of_stage | map(select(.body | test(\"Вердикт:[^\\n]*(жёлт|красн)\"; \"i\")))"
-            attempt=$(( $(printf '%s' "$comments" | jq -r "$of_stage | length") + 1 ))
-            spent=$(printf '%s' "$comments" | jq -r "$blocking | length")
-            spent_list=$(printf '%s' "$comments" | jq -r "$blocking | map(\"- \" + .url) | join(\"\\n\")")
+            comments=$(mktemp)
+            gh issue view "$NUM" --repo "$REPO" --json comments > "$comments"
 
             # Ветка задачи — та же, что выберет шаг ревью: свежая по коммиту.
             # Её нет у задач, размеченных до появления конвейера; тогда счёт по
@@ -153,16 +149,18 @@ jobs:
               gh api "repos/$REPO/contents/docs/reviews/$name?ref=$target" \
                 -H 'Accept: application/vnd.github.raw' > "$docs/$name" 2>/dev/null || rm -f "$docs/$name"
             done
+            list=$(mktemp)
             counters=$(node scripts/review-doc-guard.mjs --counters \
               --marker="$marker" --num="$NUM" --names="$names" --docs="$docs" \
-              --comment-attempt="$attempt" --comment-spent="$spent")
-            # Пустой ответ означает, что скрипт не отработал. Тогда действуют
-            # прежние значения: guard обязан продолжить работу, а не встать —
-            # худшее, что даёт откат к прозе, это сегодняшнее поведение.
+              --comments="$comments" --spent-list="$list")
+            spent_list=$(cat "$list")
+            # Пустой ответ означает, что скрипт не отработал. Тогда остаются
+            # значения по умолчанию (заход 1, циклов 0): guard обязан
+            # продолжить работу, а не встать.
             new_attempt=$(printf '%s\n' "$counters" | sed -n 's/^attempt=//p')
             new_spent=$(printf '%s\n' "$counters" | sed -n 's/^spent=//p')
             new_blocking=$(printf '%s\n' "$counters" | sed -n 's/^blocking=//p')
-            case "$new_attempt" in ''|*[!0-9]*) echo "::warning::счёт по файлам не дал числа — остаётся счёт по комментариям" ;; *) attempt="$new_attempt" ;; esac
+            case "$new_attempt" in ''|*[!0-9]*) echo "::warning::счётчик раундов не дал числа — работают значения по умолчанию" ;; *) attempt="$new_attempt" ;; esac
             case "$new_spent" in ''|*[!0-9]*) : ;; *) spent="$new_spent" ;; esac
             # Перечень учтённого обязан сходиться с числом: если цикл виден
             # только документом, ссылка на комментарий его не объяснит.

docs/specs/454-review-round-counter.md

@@ -138,9 +138,19 @@ attempt = max(attemptFromFiles, attemptFromComments)
 spent   = max(spentFromFiles,   spentFromComments)
 ```
 
-Счёт по комментариям остаётся ровно тем же, что сегодня. Недосчёт возможен
-только при отказе обоих источников; перерасчёт невозможен по построению, потому
-что берётся максимум, а не сумма.
+Недосчёт возможен только при отказе обоих источников. Удвоение невозможно по
+построению — берётся максимум, а не сумма, — но это НЕ значит, что завышение
+невозможно вовсе: максимум наследует ошибку той компоненты, которая завысила.
+Поэтому у каждого источника своё правило точности.
+
+Счёт по комментариям в первой редакции ТЗ предполагался неизменным. Ревью
+реализации показало, что оставлять его нельзя: правило «в теле есть подстрока
+`Вердикт:` и подстрока маркера» протекает на прозе. Поймано на самой #454 —
+разбор чужих задач в комментарии содержал `CODE-REVIEW`, и первый же код-ревью
+получил заход r3 вместо r1. Поэтому комментарий засчитывается вердиктом этапа,
+только если он (а) объявляет вердикт той же строгой строкой, что и документ, и
+(б) называет документ ЭТОЙ задачи и ЭТОГО этапа — `<MARKER>-<NUM>`. Голая
+подстрока маркера больше не годится: именно она и протекала.
 
 ### 4. Ветка задачи
 
@@ -178,7 +188,7 @@ spent   = max(spentFromFiles,   spentFromComments)
 | AC2b | На той же истории, прожитой уже с исправлением, ничего не теряется: фикстура трёх файлов `r1` (жёлтый), `r2` (жёлтый), `r3` (зелёный) даёт `attempt=4`, `spent=2` | unit на реконструированной фикстуре |
 | AC3 | Имя документа никогда не повторяет уже существующее на ветке | unit + мутант |
 | AC4 | Зелёный вердикт цикла не тратит (#227 не сломан) | unit |
-| AC5 | Вердикт чужого этапа не влияет на счёт (#89 не сломан) | unit |
+| AC5 | Вердикт чужого этапа не влияет на счёт (#89 не сломан) — ни по имени файла, ни по прозе комментария | unit + мутант |
 | AC6 | Отказ публикации (документа нет при существующем вердикте) не занижает счёт — работает максимум | unit |
 | AC7 | Отсутствие ветки/недоступность API не роняет `guard` и не меняет сегодняшнего поведения | unit |
 | AC8 | `process.yml` идентичен в `main` и `dev` | существующий шаг Validate |
@@ -197,8 +207,14 @@ spent   = max(spentFromFiles,   spentFromComments)
     вместо максимума; краснеет AC1/AC3;
   - `review-round-drops-file-source` — счёт по файлам выключен, остаётся
     только проза; краснеет AC2;
-  - `review-round-takes-comments-over-max` — вместо максимума берётся счёт по
-    комментариям; краснеет AC6.
+  - `review-round-drops-comment-insurance` — вместо максимума берётся счёт
+    только по файлам; краснеет AC6. (В первой редакции ТЗ мутант назывался
+    `review-round-takes-comments-over-max`; в таком виде он был неотличим от
+    предыдущего — обе мутации дают результат, равный счёту по комментариям, — и
+    AC6 не краснил, потому что там комментарии как раз больше файлов. Ломать
+    страховку нужно противоположной мутацией.)
+  - `review-comment-source-ignores-issue-number` — резервный матч по голому
+    маркеру вместо `<MARKER>-<NUM>`; краснеет AC5.
 - Ручная проверка на самом себе: эта задача проходит spec-review и code-review
   штатным конвейером; после её мержа номера документов #454 обязаны идти
   подряд.

scripts/mutation-gate.mjs

@@ -2439,6 +2439,20 @@ const MUTANT_DEFINITIONS = [
       replace: '  if (true) return null;',
     }],
   },
+  {
+    id: 'review-comment-source-ignores-issue-number',
+    guard: 'node --test --test-name-pattern="по документу ЭТОЙ задачи|чужой номер задачи" '
+      + 'test/review-doc-guard.test.mjs',
+    because: 'matching a bare stage marker counts any comment that merely mentions another '
+      + "issue's review document as a verdict of this one — #454 gave its own first code "
+      + 'review round number r3 that way, and a contaminated yellow would burn the §4 budget '
+      + 'without a single real cycle (#89)',
+    patches: [{
+      file: 'scripts/review-doc-guard.mjs',
+      find: '  const own = new RegExp(`${marker}-${num}(?![0-9])`);',
+      replace: '  const own = new RegExp(marker);',
+    }],
+  },
   {
     id: 'review-round-counts-files-not-max',
     guard: 'node --test --test-name-pattern="от максимума номеров" test/review-doc-guard.test.mjs',

scripts/review-doc-guard.mjs

@@ -449,6 +449,47 @@ export function blockingFromDocs(docs) {
   return { blocking, unread };
 }
 
+/**
+ * Вердикты этапа среди комментариев issue — вторая, страховочная половина
+ * счёта (#454).
+ *
+ * Прежнее правило было двумя тестами подстроки по всему телу: `Вердикт:` и имя
+ * маркера. Оба ловят прозу. Поймано на самой этой задаче: guard кода #454
+ * насчитал заход r3 при первом же код-ревью, потому что маркер `CODE-REVIEW`
+ * случайно встретился в зелёном вердикте СПЕК-ревью и в комментарии-передаче
+ * работы — оба разбирали историю чужих задач и цитировали имена их документов.
+ * Завышение здесь опаснее занижения: попади заражающий вердикт в свой этап
+ * жёлтым, бюджет §4 сгорел бы без единого настоящего цикла — ровно вред
+ * класса #89, от которого этап и отделяли.
+ *
+ * Поэтому два условия вместо двух подстрок:
+ *
+ * - комментарий ОБЪЯВЛЯЕТ вердикт (та же строгая строка, что и в документе), а
+ *   не упоминает слово «вердикт» в разборе;
+ * - он называет документ ЭТОЙ задачи и ЭТОГО этапа: `<MARKER>-<NUM>`. Голое
+ *   имя маркера больше не годится — именно оно и протекало.
+ */
+export function stageVerdictComments(comments, marker, num) {
+  if (!/^[A-Z-]+$/.test(String(marker || '')) || !/^\d+$/.test(String(num || ''))) return [];
+  const own = new RegExp(`${marker}-${num}(?![0-9])`);
+  return (comments || [])
+    .map((item) => (typeof item === 'string' ? { body: item } : (item || {})))
+    .filter((item) => own.test(String(item.body ?? '')))
+    .map((item) => ({ ...item, verdict: verdictDeclaration(item.body) }))
+    .filter((item) => Boolean(item.verdict));
+}
+
+/** Счётчики по комментариям в том же виде, в каком их даёт файловая половина. */
+export function commentCounters(comments, marker, num) {
+  const verdicts = stageVerdictComments(comments, marker, num);
+  const blocking = verdicts.filter((item) => isBlockingVerdict(item.verdict));
+  return {
+    attempt: verdicts.length + 1,
+    spent: blocking.length,
+    list: blocking.map((item) => item.url).filter(Boolean),
+  };
+}
+
 /**
  * Итоговые счётчики: максимум двух независимых источников.
  *
@@ -516,16 +557,37 @@ if (invokedDirectly) {
           + ` (${error.code || error.message}) — цикл по нему не засчитан`);
       }
     }
-    const counters = reviewCounters({
-      rounds,
-      docs,
-      comments: { attempt: Number(value('comment-attempt', '1')), spent: Number(value('comment-spent', '0')) },
-    });
+    // Комментарии — вторая половина счёта. Разбор их тоже здесь, а не в jq:
+    // прежнее правило «подстрока в теле» протекало на прозе (см.
+    // stageVerdictComments), и чинить его в inline-shell означало бы снова
+    // оставить счёт без единого теста.
+    let fromComments = { attempt: Number(value('comment-attempt', '1')),
+      spent: Number(value('comment-spent', '0')), list: [] };
+    const commentsFile = value('comments');
+    if (commentsFile) {
+      try {
+        const payload = JSON.parse(readFileSync(commentsFile, 'utf8'));
+        fromComments = commentCounters(payload.comments || payload, marker, num);
+      } catch (error) {
+        console.error(`::warning::комментарии не разобраны (${error.code || error.message})`
+          + ' — счёт по комментариям отключён, работает счёт по документам');
+        fromComments = { attempt: 1, spent: 0, list: [] };
+      }
+    }
+    const counters = reviewCounters({ rounds, docs, comments: fromComments });
     if (counters.unread.length) {
       console.error(`::warning::вердикт не объявлен машиночитаемой строкой в:`
         + ` ${counters.unread.join(', ')} — цикл по этим документам считается`
         + ' только по комментариям');
     }
+    // Расхождение источников — не отказ, но и не рутина: оно означает, что один
+    // из них чего-то не видит. Пусть это будет видно в прогоне, а не только в
+    // арифметике максимума.
+    if (counters.attemptComments !== counters.attemptFiles) {
+      console.error('::warning::источники счёта расходятся: по документам заход'
+        + ` ${counters.attemptFiles}, по комментариям ${counters.attemptComments}.`
+        + ' Взят максимум. Документы надёжнее: их имена собирает конвейер.');
+    }
     console.error(`::notice::раунды по файлам ${rounds.length ? rounds.join(',') : '—'};`
       + ` заход: файлы ${counters.attemptFiles}, комментарии ${counters.attemptComments};`
       + ` циклы: файлы ${counters.spentFiles}, комментарии ${counters.spentComments}`);
@@ -534,6 +596,14 @@ if (invokedDirectly) {
     // Перечень учтённого по файлам: без него владелец видит число, но не может
     // сверить, из чего оно сложилось, когда комментарии цикла не показывают.
     console.log(`blocking=${counters.blocking.join(', ')}`);
+    const listFile = value('spent-list');
+    if (listFile) {
+      try {
+        writeFileSync(listFile, fromComments.list.map((url) => `- ${url}`).join('\n'), 'utf8');
+      } catch (error) {
+        console.error(`::warning::перечень учтённых вердиктов не записан (${error.code || error.message})`);
+      }
+    }
     process.exit(0);
   }
 

test/review-doc-guard.test.mjs

@@ -5,6 +5,7 @@ import { readFileSync } from 'node:fs';
 import {
   ANCHOR_MARKER, REVIEW_DOC_ALLOWLIST, anchorLiveness, REVIEW_HEADER_LINES, citedMaterialShas, danglingMaterialRefusal, materialAnchorBlock, materialAnchorsFrom, parseSpecList, pathsOutsideAllowlist, reviewDocPushRefusal, withMaterialAnchors,
   attemptFromRounds, blockingFromDocs, isBlockingVerdict, reviewCounters, reviewRoundsFromFiles, verdictDeclaration,
+  commentCounters, stageVerdictComments,
 } from '../scripts/review-doc-guard.mjs';
 
 // #365. 28.08 шаг публикации ревью-дока запушил в dev коммит bb2919f с тридцатью
@@ -477,3 +478,51 @@ test('описание чужого раунда не объявляет вер
   assert.ok(verdictDeclaration('- **Вердикт:** жёлтый'));
   assert.ok(verdictDeclaration('- Вердикт: зелёный'));
 });
+
+// Вторая половина счёта — комментарии. Прежнее правило («в теле есть
+// «Вердикт:» и есть имя маркера») протекало на прозе, и поймано это было на
+// самой #454: разбор чужих задач в комментарии-передаче работы содержал слово
+// CODE-REVIEW, и первый же код-ревью получил заход r3 вместо r1.
+
+const C = (body, url = 'https://x/1') => ({ body, url });
+
+test('вердикт этапа опознаётся по документу ЭТОЙ задачи (#454 AC5, #89)', () => {
+  const comments = [
+    C('Вердикт: жёлтый · заход r1\n\nДокумент: docs/reviews/CODE-REVIEW-454-r1.md'),
+    // Зелёный вердикт СПЕК-этапа, случайно упомянувший чужой документ.
+    C('Вердикт: зелёный · заход r2 — разобрано по аналогии с CODE-REVIEW-441-r1.md'
+      + '\n\nДокумент: docs/reviews/SPEC-REVIEW-454-r2.md'),
+    // Передача работы: слово «Вердикт:» в прозе и имя чужого документа.
+    C('Реализация готова. Строки `Вердикт:` в документе нет; ср. CODE-REVIEW-439-r1.md'),
+  ];
+  const code = commentCounters(comments, 'CODE-REVIEW', '454');
+  assert.equal(code.attempt, 2, 'настоящий код-вердикт ровно один');
+  assert.equal(code.spent, 1);
+  const spec = commentCounters(comments, 'SPEC-REVIEW', '454');
+  assert.equal(spec.attempt, 2);
+  assert.equal(spec.spent, 0, 'зелёный цикла не тратит');
+});
+
+test('чужой номер задачи не засчитывается своим (#454 AC5)', () => {
+  const comments = [C('Вердикт: красный · заход r1\n\nДокумент: docs/reviews/CODE-REVIEW-4540-r1.md')];
+  assert.equal(commentCounters(comments, 'CODE-REVIEW', '454').attempt, 1);
+  assert.equal(commentCounters(comments, 'CODE-REVIEW', '4540').attempt, 2);
+});
+
+test('перечень учтённого сходится с числом циклов (#454)', () => {
+  const comments = [
+    C('Вердикт: жёлтый · заход r1 — CODE-REVIEW-454-r1.md', 'https://x/1'),
+    C('Вердикт: зелёный · заход r2 — CODE-REVIEW-454-r2.md', 'https://x/2'),
+    C('Вердикт: красный · заход r3 — CODE-REVIEW-454-r3.md', 'https://x/3'),
+  ];
+  const counters = commentCounters(comments, 'CODE-REVIEW', '454');
+  assert.equal(counters.spent, 2);
+  assert.deepEqual(counters.list, ['https://x/1', 'https://x/3']);
+});
+
+test('мусор в комментариях не роняет счёт (#454 AC7)', () => {
+  assert.equal(commentCounters(null, 'CODE-REVIEW', '454').attempt, 1);
+  assert.equal(commentCounters([{}, { body: null }, 'строка'], 'CODE-REVIEW', '454').attempt, 1);
+  assert.equal(commentCounters([C('Вердикт: жёлтый CODE-REVIEW-454-r1.md')], '', '454').attempt, 1);
+  assert.deepEqual(stageVerdictComments([C('Вердикт: жёлтый CODE-REVIEW-454-r1.md')], 'CODE-REVIEW', 'x'), []);
+});