fix(review): scope the bare-score public-safety check per repo (metagraphed#8038)

src/env.d.ts

@@ -316,6 +316,13 @@ declare global {
      *  non-repo-scoped contributor/operator tools (another contributor's private data, fleet analytics). Unset
      *  ⇒ none. A separate allowlist from MCP_ACTUATION_REPO_ALLOWLIST so read and write trust can differ (#2455). */
     MCP_READ_REPO_ALLOWLIST?: string;
+    /** Repos where a bare "score"/"scores"/"scored" mention in AI-review-composed public text is safe to
+     *  publish (comma/whitespace `owner/repo` list; no `*`/`all` wildcard -- see
+     *  isPublicScoreTermSafeForRepo's own doc comment for why). Unset ⇒ none: the check stays enforced
+     *  everywhere by default. Set this for a repo whose OWN public schema legitimately names fields like
+     *  `totalScore`/`credibility` (e.g. metagraphed) so a real AI review of that code doesn't have its whole
+     *  narrative assessment silently discarded (#public-score-terms-scoping). */
+    LOOPOVER_PUBLIC_SCORE_TERMS_ALLOWED_REPOS?: string;
     /** Shared bearer secret required by the hosted Orb ingest collector. */
     ORB_INGEST_TOKEN?: string;
     /** Shared bearer secret required by the hosted AMS ingest collector (#5681) — same optional, fail-open

src/queue-intelligence.ts

@@ -161,15 +161,45 @@ export async function analyzePRQueue(
   return { rankedPRs: scored.map(({ pr }) => pr), recommendations };
 }
 
-export function sanitizePublicComment(comment: string): string {
+/** CSV/whitespace repo allowlist for {@link isPublicScoreTermSafeForRepo} -- same fail-closed/wildcard parse
+ *  shape as auth/security.ts's MCP repo allowlists (a distinct, smaller local copy rather than a shared
+ *  import: this one governs a content-safety exemption, not an actuation/read trust boundary, and the two
+ *  concerns should stay independently editable). Unset/empty ⇒ deny (fail closed: the bare-score check stays
+ *  ENFORCED for every repo unless an operator explicitly opts one in). `*`/`all` is deliberately NOT treated
+ *  as a wildcard here, unlike the MCP allowlists -- this exemption is safety-relevant enough that a blanket
+ *  opt-in-everything value would silently cover a FUTURE repo that DOES have private trust/reward data (e.g.
+ *  loopover itself), which the MCP read/actuation allowlists' analogous risk doesn't share.
+ *  LOOPOVER_PUBLIC_SCORE_TERMS_ALLOWED_REPOS is the env var; config-as-code, never hardcoded per #config-as-code. */
+export function isPublicScoreTermSafeForRepo(env: { LOOPOVER_PUBLIC_SCORE_TERMS_ALLOWED_REPOS?: string }, repoFullName: string): boolean {
+  const entries = (env.LOOPOVER_PUBLIC_SCORE_TERMS_ALLOWED_REPOS ?? "")
+    .split(/[\s,]+/)
+    .map((entry) => entry.trim().toLowerCase())
+    .filter(Boolean);
+  return entries.includes(repoFullName.toLowerCase());
+}
+
+/** `allowBareScoreTerm` (#public-score-terms-scoping, default false = today's unchanged behavior): the bare
+ *  "\bscore\w*\b" check exists to catch a LEAKED gittensor trust/reward VALUE, but it also matches the word
+ *  "score" used as ordinary, already-public API vocabulary -- a real problem for a repo like metagraphed,
+ *  whose own public schema legitimately has fields named `totalScore`/`credibility`/`baseTotalScore`. Any AI
+ *  review discussing that code naturally uses the word "score" and, before this option existed, had its
+ *  WHOLE narrative assessment silently discarded (observed live: metagraphed#8038 and recurring). The other,
+ *  EXPLICIT-PHRASE entries in FORBIDDEN_PUBLIC_COMMENT_WORDS ("trust score", "reward", "scoreability", ...)
+ *  are NEVER skippable by this flag -- those name the actual private concept regardless of repo, so they stay
+ *  enforced everywhere; only the over-broad bare-word check is ever relaxed, and only where a caller has
+ *  positively confirmed (via a repo allowlist, never a blanket default) that "score" is safe public
+ *  vocabulary for that repo. */
+export function sanitizePublicComment(comment: string, options?: { allowBareScoreTerm?: boolean }): string {
   for (const forbiddenWord of FORBIDDEN_PUBLIC_COMMENT_WORDS) {
     if (comment.toLowerCase().includes(forbiddenWord.toLowerCase())) {
       throw new Error(`Public comment contains forbidden word: "${forbiddenWord}"`);
     }
   }
-  const bareScoreMatch = comment.match(BARE_SCORE_TERM_PATTERN);
-  if (bareScoreMatch) {
-    throw new Error(`Public comment contains forbidden word: "${bareScoreMatch[0]}"`);
+  if (!options?.allowBareScoreTerm) {
+    const bareScoreMatch = comment.match(BARE_SCORE_TERM_PATTERN);
+    if (bareScoreMatch) {
+      throw new Error(`Public comment contains forbidden word: "${bareScoreMatch[0]}"`);
+    }
   }
   return comment;
 }

src/queue/processors.ts

@@ -3330,6 +3330,13 @@ async function runAgentMaintenancePlanAndExecute(
         finalActionClasses: breakerOnPlan.map((action) => action.actionClass),
       },
     }).catch(() => undefined);
+    // #merge-race guarantee: "unknown" is the ONE genuinely transient mergeableState value (GitHub still
+    // computing) -- "dirty"/"blocked"/"behind" are real, stable holds that a re-check moments later would only
+    // reproduce, so they're deliberately NOT retried here. Best-effort: an enqueue failure here must never
+    // fail this webhook pass, matching every other trailing-schedule call site in this file.
+    if ((liveMergeState ?? pr.mergeableState) === "unknown") {
+      await scheduleTrailingMergeableStateReReview(env, deliveryId, installationId, repoFullName, pr.number).catch(() => undefined);
+    }
   }
   if (breakerOnPlan.length === 0) {
     return;
@@ -4344,6 +4351,49 @@ async function scheduleTrailingIssueLinkedReReview(
   await putTransientKey(env, key, "1", CI_COALESCE_WINDOW_SECONDS);
 }
 
+// #merge-race guarantee (metagraphed#8037): refreshLiveMergeState's own short inline retry (ci-resolution.ts,
+// ~4s total) resolves most GitHub mergeable_state "still computing" reads within the SAME pass, but not all --
+// on a large PR or under load, GitHub can take longer. Without this, the ONLY remaining catch-up mechanism was
+// the periodic agent-regate-sweep, which is NOT a reliable fixed interval: it throttles under GitHub REST-budget
+// backpressure and skips re-arming while a previous sweep trigger is still in flight (see the backpressure
+// comment in index.ts) -- under sustained cross-repo load this can stretch well past minutes, during which an
+// overlapping sibling PR can land first and base-conflict the original PR out from under it (observed live).
+// This schedules ONE targeted, single-PR follow-up -- reusing the same agent-regate-pr job every other trailing
+// re-check in this file uses -- completely independent of the sweep's own budget/backlog throttling, so a hold
+// caused SPECIFICALLY by an unresolved mergeable_state is guaranteed a fresh look within a short, bounded,
+// sweep-independent window instead of an unbounded one.
+const MERGE_STATE_UNKNOWN_TRAILING_RECHECK_DELAY_SECONDS = 60;
+async function scheduleTrailingMergeableStateReReview(
+  env: Env,
+  deliveryId: string,
+  installationId: number,
+  repoFullName: string,
+  prNumber: number,
+): Promise<void> {
+  const key = `merge-state-unknown-trailing:${repoFullName.toLowerCase()}#${prNumber}`;
+  // Same check-then-claim-only-after-send shape as scheduleTrailingIssueLinkedReReview above (#2371 follow-up
+  // reasoning applies identically here): claiming before the enqueue succeeds would permanently swallow the
+  // guarantee this function exists to provide for the rest of the window.
+  if (await getTransientKey(env, key)) return;
+  try {
+    await env.JOBS.send(
+      { type: "agent-regate-pr", deliveryId, repoFullName, prNumber, installationId },
+      { delaySeconds: MERGE_STATE_UNKNOWN_TRAILING_RECHECK_DELAY_SECONDS },
+    );
+  } catch (error) {
+    console.log(
+      JSON.stringify({
+        event: "merge_state_unknown_trailing_enqueue_failed",
+        repoFullName,
+        pull: prNumber,
+        message: errorMessage(error).slice(0, 120),
+      }),
+    );
+    return; // do NOT claim -- a later pass hitting the same "unknown" read should retry the enqueue
+  }
+  await putTransientKey(env, key, "1", MERGE_STATE_UNKNOWN_TRAILING_RECHECK_DELAY_SECONDS);
+}
+
 /** Best-effort wake for sibling PRs discovered to be over the per-contributor cap by a LATER delivery (#2270,
  *  #2479 gate finding): webhook delivery order isn't guaranteed to match PR creation order, so a sibling's own
  *  webhook can fire before this one exists in the DB and wrongly conclude the author is within the cap — with

src/services/ai-review.ts

@@ -24,7 +24,7 @@ import {
   recordAiUsageEvent,
   sumAiEstimatedNeuronsSince,
 } from "../db/repositories";
-import { sanitizePublicComment } from "../queue-intelligence";
+import { isPublicScoreTermSafeForRepo, sanitizePublicComment } from "../queue-intelligence";
 import { defangReviewInput } from "../review/safety";
 import { convergedFeatureActive } from "../review/feature-activation";
 import { labelSelfHostReviewerModels, labelSelfHostReviewerNames, resolveConfiguredProviderNames } from "../selfhost/ai-config";
@@ -599,12 +599,14 @@ function neutralizePublicMarkdown(text: string): string {
     .replace(/([\\`*_{}\[\]()#+!|])/g, "\\$1");
 }
 
-/** Returns neutralized text if it is public-safe, otherwise null (drop — never publish). */
-export function toPublicSafe(text: string | null | undefined): string | null {
+/** Returns neutralized text if it is public-safe, otherwise null (drop — never publish). `allowBareScoreTerm`
+ *  defaults false (unchanged behavior) -- only composeAdvisoryNotes' repo-scoped call sites ever pass true;
+ *  see sanitizePublicComment's own doc comment for why this is the ONLY relaxable check. */
+export function toPublicSafe(text: string | null | undefined, options?: { allowBareScoreTerm?: boolean }): string | null {
   const trimmed = (text ?? "").trim();
   if (!trimmed) return null;
   try {
-    return neutralizePublicMarkdown(sanitizePublicComment(trimmed));
+    return neutralizePublicMarkdown(sanitizePublicComment(trimmed, options));
   } catch {
     return null;
   }
@@ -1619,8 +1621,14 @@ function composeFallbackAdvisoryNotes(notes: readonly string[]): string | null {
   return safeNotes.join("\n\n");
 }
 
-/** Compose a public-safe markdown advisory blurb from one or two model reviews. Null if no assessment is safe. */
-export function composeAdvisoryNotes(reviews: ModelReview[]): string | null {
+/** Compose a public-safe markdown advisory blurb from one or two model reviews. Null if no assessment is safe.
+ *  `allowBareScoreTerm` (#public-score-terms-scoping, default false): set true only for a repo the caller has
+ *  confirmed via isPublicScoreTermSafeForRepo has no private trust/reward data of its own -- see
+ *  sanitizePublicComment's doc comment. Metagraphed#8038-class bug: without this, ANY review of code with a
+ *  legitimately-public `score`-named field (metagraphed's own `totalScore`/`credibility`) had its entire
+ *  narrative assessment silently discarded in favor of the generic "did not include a separate narrative
+ *  summary" fallback -- observed live, recurring. */
+export function composeAdvisoryNotes(reviews: ModelReview[], options?: { allowBareScoreTerm?: boolean }): string | null {
   const assessments = reviews.map((r) => r.assessment).filter(Boolean);
   // High-signal caps: a focused review shows only the few findings that matter (the prompt also asks the
   // model to be selective + deduplicate). Keep the core blockers and a handful of nits. (#focused-reviews)
@@ -1629,12 +1637,12 @@ export function composeAdvisoryNotes(reviews: ModelReview[]): string | null {
   const nits = [
     ...new Set(reviews.flatMap((r) => [...r.nits, ...r.suggestions])),
   ].slice(0, 5);
-  const assessment = toPublicSafe(assessments[0] ?? "");
+  const assessment = toPublicSafe(assessments[0] ?? "", options);
   const safeBlockers = blockers
-    .map((s) => toPublicSafe(s))
+    .map((s) => toPublicSafe(s, options))
     .filter((s): s is string => Boolean(s));
   const safeNits = nits
-    .map((s) => toPublicSafe(s))
+    .map((s) => toPublicSafe(s, options))
     .filter((s): s is string => Boolean(s));
   const publicAssessment =
     assessment || fallbackPublicAssessment(safeBlockers, safeNits);
@@ -2526,7 +2534,7 @@ export async function runLoopOverAiReview(
   if (inconclusive) incr("loopover_ai_review_inconclusive_total", { mode: input.mode });
   const advisoryNotes =
     reviewsForNotes.length > 0
-      ? (composeAdvisoryNotes(reviewsForNotes) ?? composeFallbackAdvisoryNotes(fallbackNotes))
+      ? (composeAdvisoryNotes(reviewsForNotes, { allowBareScoreTerm: isPublicScoreTermSafeForRepo(env, input.repoFullName) }) ?? composeFallbackAdvisoryNotes(fallbackNotes))
       : composeFallbackAdvisoryNotes(fallbackNotes);
   // Line-anchored inline findings (#inline-comments): only propagate model output when the resolved feature gate
   // asked for it. AI output is PR-author-influenced, so the prompt suffix is not an authorization boundary.

test/unit/ai-review.test.ts

@@ -3530,6 +3530,99 @@ describe("pure helpers", () => {
     expect(withNits).toContain("Add coverage for the edge case.");
   });
 
+  it("REGRESSION (#public-score-terms-scoping, metagraphed#8038): a bare 'score' mention drops the whole assessment by default, but survives when the repo is explicitly allowlisted", () => {
+    const reviewMentioningScore = [
+      {
+        // A standalone "score" in natural review prose (\bscore\w*\b -- note this does NOT match a camelCase
+        // identifier like "totalScore", only a real word-boundary-delimited mention; this is exactly the shape
+        // an AI reviewer's own natural-language description of a scoring field takes).
+        assessment: "The resolver correctly filters results by their score before returning them.",
+        suggestions: [],
+        // Deliberately score-free, unlike the assessment above: proves the fallback text below comes from
+        // this SURVIVING nit (dropping the assessment must not also silently empty out unrelated findings).
+        nits: ["Consider extracting the filter helper for reuse."],
+        blockers: [],
+        inlineFindings: [],
+        confidence: 1,
+      },
+    ];
+    // Default (no options / allowBareScoreTerm false): unchanged, current behavior -- the whole assessment is
+    // dropped and the generic no-narrative-summary fallback takes over, same as before this fix existed.
+    const defaultResult = composeAdvisoryNotes(reviewMentioningScore);
+    expect(defaultResult).toContain("did not include a separate narrative summary");
+    expect(defaultResult).not.toContain("filters results by their score");
+    // Allowlisted (what runLoopOverAiReview now passes for a repo in LOOPOVER_PUBLIC_SCORE_TERMS_ALLOWED_REPOS):
+    // the real narrative assessment survives.
+    const allowedResult = composeAdvisoryNotes(reviewMentioningScore, { allowBareScoreTerm: true });
+    expect(allowedResult).toContain("The resolver correctly filters results by their score");
+    expect(allowedResult).not.toContain("did not include a separate narrative summary");
+  });
+
+  it("REGRESSION (#public-score-terms-scoping): the EXPLICIT-PHRASE bans (trust score, reward, scoreability, ...) stay enforced even when allowBareScoreTerm is true", () => {
+    const reviewLeakingTrustScore = [
+      {
+        assessment: "This PR exposes the miner's raw trust score in the response payload.",
+        suggestions: [],
+        // A surviving, unrelated nit -- proves the explicit-phrase throw takes down ONLY the assessment
+        // (the same "dropping one field must not silently empty everything else" property as the sibling
+        // test above), not that the whole review vanishes.
+        nits: ["Add a test for the pagination edge case."],
+        blockers: [],
+        inlineFindings: [],
+        confidence: 1,
+      },
+    ];
+    const result = composeAdvisoryNotes(reviewLeakingTrustScore, { allowBareScoreTerm: true });
+    expect(result).not.toContain("raw trust score");
+    expect(result).toContain("did not include a separate narrative summary");
+    expect(result).toContain("Add a test for the pagination edge case.");
+  });
+
+  it("REGRESSION (#public-score-terms-scoping): runLoopOverAiReview resolves isPublicScoreTermSafeForRepo from LOOPOVER_PUBLIC_SCORE_TERMS_ALLOWED_REPOS and threads it into composeAdvisoryNotes end-to-end", async () => {
+    const run = vi.fn(async () => ({
+      response: reviewJson({ assessment: "The resolver correctly filters results by their score before returning them." }),
+    }));
+    const allowedEnv = createTestEnv({
+      AI: { run } as unknown as Ai,
+      AI_SUMMARIES_ENABLED: "true",
+      AI_PUBLIC_COMMENTS_ENABLED: "true",
+      LOOPOVER_PUBLIC_SCORE_TERMS_ALLOWED_REPOS: "acme/widgets",
+    });
+    const allowedResult = await runLoopOverAiReview(allowedEnv, baseInput); // baseInput.repoFullName === "acme/widgets"
+    expect(allowedResult.status).toBe("ok");
+    expect(allowedResult.status === "ok" ? allowedResult.advisoryNotes : undefined).toContain(
+      "The resolver correctly filters results by their score",
+    );
+
+    const deniedEnv = createTestEnv({
+      AI: { run } as unknown as Ai,
+      AI_SUMMARIES_ENABLED: "true",
+      AI_PUBLIC_COMMENTS_ENABLED: "true",
+      // Unset LOOPOVER_PUBLIC_SCORE_TERMS_ALLOWED_REPOS: fail-closed default, same repo as above.
+    });
+    const deniedResult = await runLoopOverAiReview(deniedEnv, baseInput);
+    expect(deniedResult.status).toBe("ok");
+    expect(deniedResult.status === "ok" ? deniedResult.advisoryNotes : undefined).not.toContain("filters results by their score");
+  });
+
+  it("REGRESSION (#public-score-terms-scoping, branch coverage): runLoopOverAiReview falls through to composeFallbackAdvisoryNotes when composeAdvisoryNotes itself returns null (every field unsafe)", async () => {
+    // Mirrors the "composeAdvisoryNotes returns null when no assessment or finding is public-safe" unit
+    // fixture, but driven end-to-end through runLoopOverAiReview so the `composeAdvisoryNotes(...) ??
+    // composeFallbackAdvisoryNotes(fallbackNotes)` line's right-hand branch is actually exercised (the
+    // score-terms tests above only ever hit composeAdvisoryNotes' own internal non-null fallback text, never
+    // this outer `??`).
+    const run = vi.fn(async () => ({
+      response: reviewJson({ assessment: "reward payout farming", suggestions: ["payout"], nits: ["reward"], blockers: [] }),
+    }));
+    const env = createTestEnv({
+      AI: { run } as unknown as Ai,
+      AI_SUMMARIES_ENABLED: "true",
+      AI_PUBLIC_COMMENTS_ENABLED: "true",
+    });
+    const result = await runLoopOverAiReview(env, baseInput);
+    expect(result.status).toBe("ok");
+  });
+
   it("parseModelReview parses well-formed inline findings, including a trimmed optional suggestion; severity defaults to nit unless exactly 'blocker' (#inline-comments)", () => {
     const json = JSON.stringify({
       assessment: "ok",