fix(triage): close the release review's sift findings

.gitignore

@@ -6,3 +6,4 @@ node_modules/
 .stryker-tmp/
 reports/mutation/
 stryker-setup-*.js
+.vitest/

README.md

@@ -138,7 +138,10 @@ a final step synthesizes one severity-ranked report.
 `workflows/triage-findings.js` gathers findings (craft agents + GitHub PR comments), sifts out what
 the project's memory already settled, validates the rest, dedups and conflict-checks them, and
 renders one ordered fix plan + a triage ledger (and the issues to file for findings the PR did not
-introduce) — no edits.
+introduce) — no edits. A finding counts as not introduced by the PR only when it was shown at the
+commit the PR forks from: pass that as `prBase` (`git merge-base origin/<trunk> <reviewed head>`);
+without it such findings come back as `needs-decision`. `base` is the separate ref the findings were
+generated against, which validation pins to.
 
 Both review workflows and the standalone agents emit a structured run record after each run; see
 `docs/observability.md`.

lib/pr-rejections.mjs

@@ -6,7 +6,7 @@
 // craft finding's thread, lib/pr-reviewer-thread.mjs for any other reviewer's (realm @nick/craft, node #302).
 import fs from 'node:fs'
 import { threadDecision, NOT_AUTHORIZED, NOT_CRAFT_ROOT } from './pr-thread-rule.mjs'
-import { reviewerThreadDecision, SELF_DISMISSED } from './pr-reviewer-thread.mjs'
+import { reviewerThreadDecision, NOT_A_MAINTAINER } from './pr-reviewer-thread.mjs'
 import { outputLines, runIfMain } from './script-main.mjs'
 
 /** @typedef {import('./pr-thread-rule.mjs').ReviewThread} ReviewThread @typedef {import('./pr-thread-rule.mjs').DecisionRecord} DecisionRecord */
@@ -70,8 +70,8 @@ export function rejectionsFromThreads(answer) {
   if (forged) skipped.push(`${forged} thread(s) not read as craft findings: the marked root comment's author is not OWNER/MEMBER/COLLABORATOR`)
   const unauthorized = skipped.filter(s => s.endsWith(NOT_AUTHORIZED)).length
   if (unauthorized) skipped.push(`${unauthorized} rejection(s) not recorded: the replier is neither the PR author nor OWNER/MEMBER/COLLABORATOR`)
-  const selfDismissed = skipped.filter(s => s.endsWith(SELF_DISMISSED)).length
-  if (selfDismissed) skipped.push(`${selfDismissed} rejection(s) not recorded: an outside reviewer rejected their own remark`)
+  const notMaintainer = skipped.filter(s => s.endsWith(NOT_A_MAINTAINER)).length
+  if (notMaintainer) skipped.push(`${notMaintainer} rejection(s) of another reviewer's remark not recorded: the PR author is not OWNER/MEMBER/COLLABORATOR`)
   const unread = Math.max(threads.total, threads.nodes.length) - read.length
   if (unread > 0) skipped.push(`${unread} thread(s) past the ${PR_THREADS_MAX} read were not examined — nothing recorded from them`)
   return { decisions, skipped }

lib/pr-reviewer-thread.mjs

@@ -1,13 +1,14 @@
 // A rejection in the thread of a reviewer that is not craft (realm @nick/craft, node #302): a person or
 // their own tool comments on the PR, the comment carries no craft-finding marker, and the last word is a
-// reply that rejects it, from the PR author or an OWNER/MEMBER/COLLABORATOR — the same reply rule as a
-// craft thread (lib/pr-thread-rule.mjs, nodes #182, #197). It becomes a decision record without asking
+// reply that rejects it, from an OWNER/MEMBER/COLLABORATOR. It becomes a decision record without asking
 // (node #223), so the same remark in the next PR is set aside by triage's sift (node #305). The root may
-// come from anyone; one guard more than a craft thread needs: an outsider who rejects their own remark
-// has settled nothing, or a contributor could post a remark and its rejection to silence a finding for
-// the whole project (the reason a forged craft root is refused, node #186). No reply is no rejection
-// (node #185). Read across a whole PR by lib/pr-rejections.mjs, after lib/pr-thread-rule.mjs says the
-// thread is not craft's.
+// come from anyone, so the reply rule is narrower than a craft thread's (lib/pr-thread-rule.mjs, nodes
+// #182, #197): there the PR author's word counts, here only a maintainer's. Logins prove nothing — an
+// outside PR author with a second account could post a remark under one and reject it under the other,
+// silencing a finding for the whole project (the reason a forged craft root is refused, node #186) — so
+// a PR author without that association never settles another reviewer's remark. No reply is no
+// rejection (node #185). Read across a whole PR by lib/pr-rejections.mjs, after lib/pr-thread-rule.mjs
+// says the thread is not craft's.
 import { REJECTING_ASSOCIATIONS, THREAD_COMMENTS_MAX, decisionId, recordProblem, threadAnchor, threadRejection } from './pr-thread-rule.mjs'
 
 /**
@@ -18,8 +19,9 @@ import { REJECTING_ASSOCIATIONS, THREAD_COMMENTS_MAX, decisionId, recordProblem,
 // A reviewer's remark has no title line of its own: its first line, cut at a word, stands in for one.
 export const REMARK_TITLE_MAX = 120
 
-// The tail of the skip line for an outsider rejecting their own remark; lib/pr-rejections.mjs counts by it.
-export const SELF_DISMISSED = 'who also wrote the remark and is not an owner, member or collaborator: not recorded'
+// The tail of the skip line for a PR author with no maintainer association rejecting another reviewer's
+// remark; lib/pr-rejections.mjs counts by it.
+export const NOT_A_MAINTAINER = "the PR author but not an owner, member or collaborator: another reviewer's remark is not theirs to settle, not recorded"
 
 /** @param {unknown} v */
 const str = v => String(v ?? '').trim()
@@ -56,13 +58,14 @@ export function remarkThread(t) {
 const remarkName = (scope, title, reviewer) => `${scope || '?'}: ${title || '(remark without text)'} (reviewer ${reviewer || '?'})`
 
 /**
- * Why a rejection cannot settle the remark, '' when it can: an outsider rejecting their own remark, or a
- * remark with no text to title the decision.
+ * Why a rejection cannot settle the remark, '' when it can: a rejecter who is not an owner, member or
+ * collaborator (the PR author, whose word counts in a craft thread only), or a remark with no text to
+ * title the decision.
  * @param {NonNullable<ReturnType<typeof remarkThread>>} c @param {string} by @returns {string}
  */
 function unsettled(c, by) {
   const assoc = str(c.cs[c.cs.length - 1]?.authorAssociation).toUpperCase()
-  if (c.reviewer && c.reviewer === by && !REJECTING_ASSOCIATIONS.includes(assoc)) return `rejected by ${by}, ${SELF_DISMISSED}`
+  if (!REJECTING_ASSOCIATIONS.includes(assoc)) return `rejected by ${by || '?'} (association ${assoc || 'unknown'}), ${NOT_A_MAINTAINER}`
   return c.title ? '' : 'the remark has no text to title the decision: record it by hand'
 }
 

lib/pr-reviewer-thread.test.mjs

@@ -1,6 +1,6 @@
 // A rejection in another reviewer's thread (realm @nick/craft, node #302): the root carries no craft marker
-// and may come from anyone; the last word rejects it, from the PR author or an owner, member or
-// collaborator; it is recorded like a craft thread's (lib/pr-thread-rule.mjs), and both kinds of thread
+// and may come from anyone; the last word rejects it, from an owner, member or collaborator — the PR
+// author only when they are one; it is recorded like a craft thread's (lib/pr-thread-rule.mjs), and both kinds of thread
 // read side by side across one PR (lib/pr-rejections.mjs). Fields as `gh api graphql` returns them.
 import { test } from 'vitest'
 import assert from 'node:assert/strict'
@@ -20,8 +20,8 @@ const answer = (nodes, prAuthor = 'carol') => ({ data: { repository: { pullReque
 /** @param {unknown} a @returns {{ decisions: any[], skipped: string[] }} */
 const read = a => /** @type {any} */ (rejectionsFromThreads(a))
 
-test('another reviewer\'s remark rejected by the PR author is a decision, titled by its first line, anchored at its line', () => {
-  const r = read(answer([thread([REMARK, comment('carol', 'CONTRIBUTOR', 'By design: the caller caps it.')])]))
+test('another reviewer\'s remark rejected by a PR author who maintains the repo is a decision, titled by its first line, anchored at its line', () => {
+  const r = read(answer([thread([REMARK, comment('carol', 'OWNER', 'By design: the caller caps it.')])]))
   assert.deepEqual(r.decisions, [{
     id: decisionId('FR-12: retry loop has no upper bound', 'src/retry.rs'), kind: 'decision', title: 'FR-12: retry loop has no upper bound',
     body: 'By design: the caller caps it.', scope: 'src/retry.rs', status: 'active', date: '2026-10-02', author: 'carol', commit: SHA,
@@ -51,18 +51,25 @@ test('conservative: no reply, a fixed reply, a later word, a drive-by rejecter,
   assert.match(driveBy.skipped.join('\n'), /1 rejection\(s\) not recorded: the replier is neither the PR author/)
   const self = none([comment('carol', 'CONTRIBUTOR', 'unbounded retry'), comment('carol', 'CONTRIBUTOR', 'Not a bug.')])
   assert.equal(self.decisions.length, 0)
-  assert.match(self.skipped.join('\n'), /rejected by carol, who also wrote the remark and is not an owner, member or collaborator: not recorded/)
-  assert.match(self.skipped.join('\n'), /1 rejection\(s\) not recorded: an outside reviewer rejected their own remark/)
+  assert.match(self.skipped.join('\n'), /rejected by carol \(association CONTRIBUTOR\), the PR author but not an owner, member or collaborator: another reviewer's remark is not theirs to settle, not recorded/)
+  assert.match(self.skipped.join('\n'), /1 rejection\(s\) of another reviewer's remark not recorded: the PR author is not OWNER\/MEMBER\/COLLABORATOR/)
+  // A second account (node #302): the remark from one login, the PR author's rejection from another.
+  const second = none([REMARK, comment('carol', 'CONTRIBUTOR', 'Not a bug.')])
+  assert.equal(second.decisions.length, 0, 'an outside PR author cannot settle another reviewer\'s remark for the project')
+  assert.match(second.skipped.join('\n'), /\(reviewer snitch-bot\) — rejected by carol \(association CONTRIBUTOR\), the PR author but not/)
+  for (const assoc of ['NONE', 'FIRST_TIME_CONTRIBUTOR', 'FIRST_TIMER', '']) {
+    assert.equal(none([REMARK, comment('carol', assoc, 'Not a bug.')]).decisions.length, 0, assoc)
+  }
   const owner = none([comment('nick', 'OWNER', 'unbounded retry'), comment('nick', 'OWNER', 'Not a bug.')], 'nick')
   assert.equal(owner.decisions.length, 1, 'a maintainer may settle their own remark')
 })
 
 test('bounds and anchors: past the comments read, no text, no commit — named, not recorded', () => {
-  const many = reviewerThreadDecision(thread([REMARK, comment('carol', 'CONTRIBUTOR', 'Not a bug.')], { comments: { totalCount: 60, nodes: [REMARK, comment('carol', 'CONTRIBUTOR', 'Not a bug.')] } }), 'carol')
+  const many = reviewerThreadDecision(thread([REMARK, comment('carol', 'OWNER', 'Not a bug.')], { comments: { totalCount: 60, nodes: [REMARK, comment('carol', 'OWNER', 'Not a bug.')] } }), 'carol')
   assert.match(String(many), /60 comments, past the 50 read/)
-  const blank = reviewerThreadDecision(thread([comment('x', 'NONE', '  \n'), comment('carol', 'CONTRIBUTOR', 'Not a bug.')]), 'carol')
+  const blank = reviewerThreadDecision(thread([comment('x', 'NONE', '  \n'), comment('carol', 'OWNER', 'Not a bug.')]), 'carol')
   assert.match(String(blank), /\(remark without text\) .* no text to title the decision/)
-  const noCommit = reviewerThreadDecision(thread([{ ...REMARK, originalCommit: null }, comment('carol', 'CONTRIBUTOR', 'Not a bug.')]), 'carol')
+  const noCommit = reviewerThreadDecision(thread([{ ...REMARK, originalCommit: null }, comment('carol', 'OWNER', 'Not a bug.')]), 'carol')
   assert.match(String(noCommit), /no path or commit to anchor the decision to/)
   assert.equal(reviewerThreadDecision({ path: 'a' }, 'carol'), null)
 })
@@ -81,7 +88,7 @@ test('both kinds of thread on one PR: a craft finding and another reviewer\'s re
   const craftRoot = comment('nick', 'OWNER', findingCommentBody({ severity: 'Medium', title: 'unwrap in parser may panic', why: 'panics', fix: 'return an error' }), 'https://x/pull/9#c1')
   const r = read(answer([
     thread([craftRoot, comment('carol', 'CONTRIBUTOR', 'Not a bug: validated upstream.')], { path: 'src/parse.rs' }),
-    thread([REMARK, comment('carol', 'CONTRIBUTOR', "Won't fix: capped by the caller.")]),
+    thread([REMARK, comment('alice', 'MEMBER', "Won't fix: capped by the caller.")]),
   ]))
   assert.deepEqual(r.decisions.map(d => [d.scope, d.title]), [['src/parse.rs', 'unwrap in parser may panic'], ['src/retry.rs', 'FR-12: retry loop has no upper bound']])
 })

lib/triage-noise-class.mjs

@@ -1,11 +1,14 @@
 // The validator's noise classes in triage-findings (realm @nick/craft, node #303), read back from its
 // answer: "not this PR's" (class 1) on a real finding is a deferral carried by an issue on the consumer's
-// forge, and only when premise_checked names what was opened at the base — a merge-base, or a
-// `git show <ref>:<path>`; else a human decides. Only an `accept` or a `defer` turns into that deferral: a
-// class-1 `reject` is not a real problem and files nothing, a class-1 `needs-decision` still waits on a human.
+// forge, and only when premise_checked names the code opened at the PR's fork point the triage was given
+// as `prBase` — `git show <prBase>:<path>`, that commit and no other ref (node #309). Not the validation
+// pin `base`: that is the ref the findings were generated against, usually the reviewed head, the PR's
+// own code. A premise naming HEAD, the pin, another ref or only the word merge-base has shown nothing,
+// and with no prBase given nothing can be shown; else a human decides. Only an `accept` or a `defer`
+// turns into that deferral: a class-1 `reject` is not a real problem and files nothing, a class-1
+// `needs-decision` still waits on a human.
 // "Not decidable from the code" (class 5) is a needs-decision.
 // Inlined into src/triage-findings.js.
-
 /** @typedef {{ severity: string, title: string, location: string, detail: string, source: string, stable_id: string }} IssueFinding */
 
 /**
@@ -17,27 +20,68 @@ export function noiseClassOf(v) {
   return Number.isInteger(n) && n >= 1 && n <= 6 ? n : 0
 }
 
+/**
+ * What the validator is told about where class 1 must be shown: the PR's fork point this triage was
+ * given, cited exactly as `classifiedVerdict` reads it, so the engine can tell the premise opened that
+ * commit rather than a ref the validator picked (node #309); with none, that a human decides.
+ * @param {string} prBase @returns {string}
+ */
+export function classOneBaseLine(prBase) {
+  const at = String(prBase ?? '').trim()
+  return at
+    ? `the PR forks from \`${at}\` (prBase) — open the cited code there and record in premise_checked exactly \`git show ${at}:<path>\`; a premise naming any other ref (HEAD, the ref you validate against, a branch, a merge-base you computed) reads as unchecked and goes to a human`
+    : "no prBase (the PR's fork point) was given to this triage, so it cannot be checked here — name the class all the same; a human decides it"
+}
+
+/**
+ * Whether a premise shows a file opened at exactly `ref`: `git show <ref>:<path>`, at the start, after
+ * a space, or inside backticks, quotes or parentheses; the ref ends at the colon, so neither another
+ * ref nor one that only starts with this one passes.
+ * @param {string} premise @param {string} ref @returns {boolean}
+ */
+export function openedAt(premise, ref) {
+  const needle = `git show ${ref}:`
+  for (let i = premise.indexOf(needle); i !== -1; i = premise.indexOf(needle, i + 1)) {
+    const before = i === 0 ? ' ' : premise.charAt(i - 1)
+    const after = premise.charAt(i + needle.length)
+    if (/[\s`'"(]/.test(before) && after !== '' && !/[\s:`'")]/.test(after)) return true
+  }
+  return false
+}
+
 /**
  * The verdict a noise class settles: class 1 (not this PR's) on a finding the validator accepted or
  * deferred is a deferral carried by an issue on the consumer's forge — but only when premise_checked
- * names the base it opened, else nothing settled it and a human decides; any other class-1 verdict
- * stands (a `reject`: no real problem, no issue; a `needs-decision`: still the human's). Class 5 (not
- * decidable from the code) is needs-decision. Every other verdict stands as the validator gave it.
+ * shows a file opened at `prBase`, the PR's fork point the triage was given, never a ref the validator
+ * picked — else nothing settled it and a human decides; with no prBase, always a human. Any other
+ * class-1 verdict stands (a `reject`: no real problem, no issue; a `needs-decision`: still the human's).
+ * Class 5 (not decidable from the code) is needs-decision. Every other verdict stands as given.
  * @template {{ verdict: string, reason: string, premise_checked: string }} V @param {V} v
+ * @param {string} [prBase] the commit the PR forks from; '' or absent when none was given
  * @returns {V & { action?: string }}
  */
-export function classifiedVerdict(v) {
+export function classifiedVerdict(v, prBase = '') {
   const n = noiseClassOf(v)
-  if (n === 1 && v.verdict !== 'accept' && v.verdict !== 'defer') return v
-  // "Checked at the base" is what premise_checked shows: the merge-base taken, or a file read at a ref.
-  if (n === 1 && !/\bmerge-base\b|\bgit show [^\s:]+:\S/.test(String(v.premise_checked ?? ''))) {
-    return { ...v, verdict: 'needs-decision', reason: `claimed not this PR's, but nothing at the base was opened to show it — ${v.reason}` }
-  }
-  if (n === 1) return { ...v, verdict: 'defer', action: 'issue' }
+  if (n === 1) return notThisPrs(v, String(prBase ?? '').trim())
   if (n === 5) return { ...v, verdict: 'needs-decision' }
   return v
 }
 
+/**
+ * A class-1 verdict settled: a real problem shown at `prBase` is a deferral with an issue, one not shown
+ * there a human's; a `reject` or `needs-decision` stands.
+ * @template {{ verdict: string, reason: string, premise_checked: string }} V @param {V} v @param {string} prBase
+ * @returns {V & { action?: string }}
+ */
+export function notThisPrs(v, prBase) {
+  if (v.verdict !== 'accept' && v.verdict !== 'defer') return v
+  if (!prBase) return { ...v, verdict: 'needs-decision', reason: `claimed not this PR's, but no prBase (the PR's fork point) was given to check it at — ${v.reason}` }
+  if (!openedAt(String(v.premise_checked ?? ''), prBase)) {
+    return { ...v, verdict: 'needs-decision', reason: `claimed not this PR's, but nothing at the PR's fork point ${prBase} was opened to show it — ${v.reason}` }
+  }
+  return { ...v, verdict: 'defer', action: 'issue' }
+}
+
 /**
  * The issue a class-1 finding becomes: its title, and a body naming where it stands, why it is not this
  * PR's, and the source's own words.