fix(advisory): rank a withdrawn finding below every finding nobody withdrew

changelog.d/fixed-2364-retracted-findings.md

@@ -0,0 +1 @@
+- The advisory digest no longer spends a top-N slot on a finding the reporting agent has already withdrawn ([#2364](https://github.com/hivecommons/hive/issues/2364)). The 2026-09-05 digest held one of its five CRITICAL slots with `heartbeatBearerOK per-hive binding removed without covering test`, whose own detail begins *"REVISED: ... not removed ... The change is refactoring, not a security regression. Downgrading priority."* — while 282 findings went unshown. "Downgrading priority" never reached the bead: the agent edited the notes, severity is read from the bead's priority, so the finding kept rendering at critical and winning a slot every cycle. A withdrawn finding now ranks below every finding nobody withdrew, in any severity band, and renders with a caption saying the bead wants closing rather than fixing. This is the only demotion that crosses severity bands: [#5945](https://github.com/hivecommons/hive/issues/5945)'s three signals all mean "nobody re-checked this", so an unverified critical may still be the worst thing in the report, whereas a retraction is the reporting agent re-checking and saying the finding does not stand. It is demoted rather than dropped, because the bead is still open and only a maintainer closes it. Detection requires two signals — a revision marker at the start of the detail AND a phrase saying the finding no longer stands — because "REVISED:" alone is equally how an agent raises a finding's severity, and demoting that would bury the most urgent item in the report.

src/pkg/advisory/advisory.go

@@ -503,6 +503,16 @@ func (o DigestOptions) resolvedRenderCap() int {
 // resolved on the findings by the time ranking runs, so only the path check
 // needs verify.
 //
+// A finding its own author has WITHDRAWN (findingRetracted) ranks below every
+// finding nobody has withdrawn, in ANY band. This is the one demotion that
+// crosses severity bands, and the reason is the same sentence that keeps the
+// others inside theirs: unverified means nobody re-checked, so an unverified
+// critical may still be the worst thing here. A retraction is the opposite --
+// the reporting agent DID re-check and said the finding does not stand -- so
+// its filed severity is the one claim it no longer makes. It still backfills
+// rather than being dropped, because the bead is open and only a maintainer
+// closes it (hivecommons/hive#2364).
+//
 // verify, when non-nil, reports whether a finding's file path still exists at
 // the analyzed snapshot. Verification is on-demand and ordered: the ranked list
 // is walked from the top and verify is called only until cap findings are in
@@ -556,6 +566,9 @@ func applyTopN(byAgent map[string][]Finding, cap int, verify func(path string) b
 	// confirmed low — demoting across bands would let a cosmetic nit displace a
 	// security finding whose file was merely renamed.
 	var kept []Finding
+	// Withdrawn findings are collected across ALL bands and placed last -- see
+	// the band-crossing paragraph in the doc comment above.
+	var retracted []Finding
 	for i := 0; i < len(all) && len(kept) < cap; {
 		rank := severityRank(all[i].Severity)
 		j := i
@@ -571,6 +584,13 @@ func applyTopN(byAgent map[string][]Finding, cap int, verify func(path string) b
 			if len(kept) == cap {
 				break
 			}
+			if findingRetracted(f) {
+				// Set aside BEFORE markPathStale: a withdrawn finding is only
+				// reached if slots go unclaimed, and verify is a lookup the
+				// ranking promises not to spend on findings it never renders.
+				retracted = append(retracted, f)
+				continue
+			}
 			markPathStale(&f)
 			if f.evidenceUnverified() {
 				unverified = append(unverified, f)
@@ -583,6 +603,15 @@ func applyTopN(byAgent map[string][]Finding, cap int, verify func(path string) b
 		}
 		i = j
 	}
+	// Nothing live is left to show: rather than render a short digest, fill the
+	// remaining slots with the withdrawn findings, in the order they ranked.
+	// They arrive carrying the renderer's withdrawal caption, which is what
+	// tells a maintainer the bead wants closing rather than fixing.
+	for k := 0; len(kept) < cap && k < len(retracted); k++ {
+		f := retracted[k]
+		markPathStale(&f)
+		kept = append(kept, f)
+	}
 
 	capped := make(map[string][]Finding, len(byAgent))
 	for _, f := range kept {
@@ -1092,7 +1121,14 @@ func FormatDigestMarkdown(d *Digest, opts DigestOptions) string {
 			// letting it cover this one is the overclaim that got a stale
 			// finding reported as a fabrication.
 			prov := ""
-			if f.ProvenanceStale && f.ProvenanceSHA != "" {
+			if findingRetracted(f) {
+				// The reporting agent withdrew this finding in the detail
+				// rendered directly below, but the bead is still open at this
+				// severity -- only a maintainer closes it. Say which of the two
+				// the reader is looking at, so the entry reads as a bead to
+				// close rather than a problem to fix (#2364).
+				prov = " ⚠️ _(withdrawn by the reporting agent in the detail below — the bead is still open at this severity, so it wants closing rather than fixing)_"
+			} else if f.ProvenanceStale && f.ProvenanceSHA != "" {
 				prov = fmt.Sprintf(" ⚠️ _(evidence computed at `%s`, not re-verified at the analyzed commit)_", shortSHA(f.ProvenanceSHA))
 			} else if f.CachedReplays > 0 && f.ProvenanceSHA == "" {
 				// A no-provenance finding whose only "confirmations" were

src/pkg/advisory/retraction.go

@@ -0,0 +1,116 @@
+package advisory
+
+import (
+	"regexp"
+	"strings"
+)
+
+// A finding can stop being worth a top-N slot in two very different ways, and
+// the digest only ever handled one of them.
+//
+// The three signals behind evidenceUnverified all say the same thing: NOBODY
+// re-checked this. The path is gone (#3704), the evidence came from another
+// commit (#5130), the repetition was cached text (#5236). None is a statement
+// about whether the finding is true, which is exactly why applyTopN demotes on
+// them WITHIN a severity band rather than across bands -- an unverified
+// critical may still be the worst thing in the report.
+//
+// This file covers the other way: the agent that filed the finding came back,
+// re-checked it, and said in the finding's own detail that it does not stand.
+// That is not an absence of verification. It is verification with a negative
+// result, from the only party that ever looked, and it was the one such signal
+// the ranking ignored entirely.
+//
+// The live case, from the 2026-09-05 digest on hivecommons/hive#2364, held one
+// of the five CRITICAL slots while 282 findings went unshown:
+//
+//	[regression-risk] heartbeatBearerOK per-hive binding removed without
+//	covering test
+//	> REVISED: Per-hive identity binding was MOVED from heartbeatBearerOK to
+//	> verifyHeartbeatBearer in hub_keys.go, not removed. ... The change is
+//	> refactoring, not a security regression. Downgrading priority.
+//
+// "Downgrading priority" never reached the bead: beadPriorityToSeverity reads
+// b.Priority, the agent edited only the notes, so the finding kept rendering at
+// critical and winning a slot every cycle. The maintainer working the tracker
+// wrote it up as "refuted by its own detail line and should be closed rather
+// than fixed -- its bead is still open at critical, which is why it keeps
+// consuming a slot".
+
+// TWO SIGNALS ARE REQUIRED, and the asymmetry is deliberate.
+//
+// "REVISED:" alone is not a withdrawal. An agent can equally write "REVISED:
+// raising this to critical after finding a second call site", and demoting THAT
+// below every other finding would bury the most urgent thing in the report
+// under a caption telling the reader to disregard it. So a finding counts as
+// retracted only when its detail opens with a revision marker AND says
+// somewhere that the finding does not stand.
+//
+// Missing a real retraction costs nothing beyond the status quo: the finding
+// keeps its filed severity, exactly as today. Inventing one costs a live
+// critical its slot. The rule fails toward KEEPING findings, and both
+// directions are tested.
+
+// retractionMarkerPattern matches an explicit revision lead-in at the very
+// START of a finding's detail: "REVISED:", "**CORRECTION:**", "> RETRACTED --".
+//
+// Anchored on purpose. A finding whose body merely mentions the word revised
+// ("the workflow was revised in #4102") is reporting something, not retracting
+// it, and only the lead-in position separates the two. The leading character
+// class absorbs the markdown and quoting agents decorate these with.
+//
+// "revision" is deliberately NOT in this list, though "revised" is. This
+// package already uses "revision <sha>" as PROVENANCE vocabulary --
+// provenanceRefPattern matches exactly that -- so a detail opening
+// "revision c9546a8 ..." states where the evidence came from rather than
+// withdrawing it. The collision is not hypothetical: provenance_test.go builds
+// a fixture that way, and it is what caught this while the rule was written.
+var retractionMarkerPattern = regexp.MustCompile(
+	"(?i)^[[:space:]>*_#-]*(revised|correction|corrected|retracted|retraction|withdrawn)\\b[[:space:]:.,-]*")
+
+// retractionWithdrawalCues are the phrases that turn a revision into a
+// withdrawal. One is enough. They are matched as substrings of the LOWERCASED
+// detail rather than as a regex, so extending the list later needs no thought
+// about escaping.
+//
+// Deliberately short: every entry is something an agent would only write about
+// a finding it no longer stands behind.
+var retractionWithdrawalCues = []string{
+	"downgrad", // "Downgrading priority", "downgraded to low"
+	"retract",  // "retracting this finding"
+	"withdraw", //
+	"false positive",
+	"not a regression",
+	"not a security regression",
+	"does not stand",
+	"no longer applies",
+	"no longer valid",
+	"was incorrect",
+	"is incorrect",
+	"not an issue",
+	"superseded",
+}
+
+// findingRetracted reports whether the agent that filed this finding has since
+// withdrawn it in the finding's own detail.
+//
+// It reads only the detail. A title is written once, when the finding is first
+// filed; the notes are where an agent goes back and revises, which is why the
+// live case says nothing about the retraction in its title and why the bead's
+// priority -- and therefore its rendered severity -- never moved.
+func findingRetracted(f Finding) bool {
+	detail := strings.TrimSpace(f.Detail)
+	if detail == "" {
+		return false
+	}
+	if !retractionMarkerPattern.MatchString(detail) {
+		return false
+	}
+	lower := strings.ToLower(detail)
+	for _, cue := range retractionWithdrawalCues {
+		if strings.Contains(lower, cue) {
+			return true
+		}
+	}
+	return false
+}