fix(gate,review,ops): add glob exclusions, tier the public-comment vocabulary, and make inert config queryable (#9554, #9555, #9433)

apps/loopover-ui/src/lib/selfhost-env-reference.ts

@@ -285,6 +285,14 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [
     name: "LOOPOVER_METRICS_REPO_LABELS",
     firstReference: "src/server.ts",
   },
+  {
+    name: "LOOPOVER_PUBLIC_SCORE_TERMS_ALLOWED_REPOS",
+    firstReference: "src/selfhost/inert-config.ts",
+  },
+  {
+    name: "LOOPOVER_PUBLIC_STATS_REPOS",
+    firstReference: "src/selfhost/inert-config.ts",
+  },
   {
     name: "LOOPOVER_REPO_CONFIG_DIR",
     firstReference: "src/server.ts",
@@ -297,6 +305,10 @@ export const SELFHOST_ENV_REFERENCE_ROWS: SelfHostEnvReferenceRow[] = [
     name: "LOOPOVER_REVIEW_RAG",
     firstReference: "src/selfhost/ai.ts",
   },
+  {
+    name: "LOOPOVER_REVIEW_SAFETY",
+    firstReference: "src/selfhost/inert-config.ts",
+  },
   {
     name: "LOOPOVER_SHUTDOWN_LOCK_RELEASE_AFTER_MS",
     firstReference: "src/server.ts",
@@ -744,9 +756,12 @@ export const SELFHOST_ENV_REFERENCE_MARKDOWN = [
   "| `LOOPOVER_ENABLE_UNSAFE_CODEX_REVIEWER` | `src/selfhost/ai.ts` |",
   "| `LOOPOVER_MCP_TOKEN` | `src/selfhost/preflight.ts` |",
   "| `LOOPOVER_METRICS_REPO_LABELS` | `src/server.ts` |",
+  "| `LOOPOVER_PUBLIC_SCORE_TERMS_ALLOWED_REPOS` | `src/selfhost/inert-config.ts` |",
+  "| `LOOPOVER_PUBLIC_STATS_REPOS` | `src/selfhost/inert-config.ts` |",
   "| `LOOPOVER_REPO_CONFIG_DIR` | `src/server.ts` |",
   "| `LOOPOVER_REVIEW_CONTINUOUS` | `src/queue/processors.ts` |",
   "| `LOOPOVER_REVIEW_RAG` | `src/selfhost/ai.ts` |",
+  "| `LOOPOVER_REVIEW_SAFETY` | `src/selfhost/inert-config.ts` |",
   "| `LOOPOVER_SHUTDOWN_LOCK_RELEASE_AFTER_MS` | `src/server.ts` |",
   "| `LOOPOVER_SINGLE_INSTANCE` | `src/selfhost/redis-cache.ts` |",
   "| `LOOPOVER_VERSION` | `src/selfhost/otel.ts` |",

packages/loopover-engine/src/review/screenshot-table-gate.ts

@@ -1,4 +1,4 @@
-import { matchesAny } from "../signals/change-guardrail.js";
+import { matchesAnyWithExclusions } from "../signals/change-guardrail.js";
 import type { ScreenshotTableGateAction, ScreenshotTableGateConfig } from "../types/manifest-deps-types.js";
 
 export type { ScreenshotTableGateAction, ScreenshotTableGateConfig } from "../types/manifest-deps-types.js";
@@ -174,7 +174,12 @@ export function hasCommittedImageFile(changedFiles: string[], scopedPaths: strin
   return changedFiles.some((file) => {
     const lower = file.toLowerCase();
     if (!IMAGE_EXTENSIONS.some((ext) => lower.endsWith(ext))) return false;
-    return scopedPaths.length === 0 || matchesAny(file, scopedPaths);
+    // #9434: exclusion-aware, so an operator can scope "this directory except these generated files"
+    // (e.g. "apps/loopover-ui/public/**" minus "!apps/loopover-ui/public/openapi.json") instead of being
+    // forced to enumerate every safe subpath. Must stay the SAME matcher isScreenshotTableGateInScope uses
+    // below -- both read config.whenPaths, and disagreeing on which paths count as "scoped" would make a
+    // path excluded from gate SCOPE still count as a stray committed image, or vice versa.
+    return scopedPaths.length === 0 || matchesAnyWithExclusions(file, scopedPaths);
   });
 }
 
@@ -312,7 +317,8 @@ export function isScreenshotTableGateInScope(config: ScreenshotTableGateConfig,
   if (config.whenLabels.length === 0 && config.whenPaths.length === 0) return true;
   const wantedLabels = new Set(config.whenLabels.map((label) => label.toLowerCase()));
   const labelMatch = config.whenLabels.length > 0 && prLabels.some((label) => wantedLabels.has(label.toLowerCase()));
-  const pathMatch = config.whenPaths.length > 0 && changedFiles.some((file) => matchesAny(file, config.whenPaths));
+  // #9434: exclusion-aware -- see hasCommittedImageFile's own comment on why the two must share one matcher.
+  const pathMatch = config.whenPaths.length > 0 && changedFiles.some((file) => matchesAnyWithExclusions(file, config.whenPaths));
   return labelMatch || pathMatch;
 }
 

packages/loopover-engine/src/signals/change-guardrail.ts

@@ -116,6 +116,47 @@ export function matchesAny(path: string, globs: string[]): boolean {
   return globs.some((g) => hasUnsafeWildcardCount(g) || globToRegExp(g).test(canonicalPath));
 }
 
+/**
+ * #9434: `matchesAny` is deliberately positive-only — there is no way to say "this directory, except these
+ * generated files" without enumerating every safe subpath by hand. That gap is exactly what let
+ * `apps/loopover-ui/public/**` (a directory that is overwhelmingly non-visual: a generated openapi.json,
+ * robots.txt, sitemap.xml, favicons) sit in a screenshotTableGate `whenPaths` list and auto-close 5
+ * contributor PRs one-shot for regenerating a JSON spec file, with zero recovery — a one-shot close has none.
+ * Confirmed the identical glob shape independently present in two sibling repos' private configs.
+ *
+ * A glob prefixed with `!` here is an EXCLUSION: a path counts as matched only if it hits at least one
+ * INCLUDE glob and hits NO exclude glob. This is deliberately a SEPARATE function, not a change to
+ * `matchesAny` itself — `matchesAny` is also the hard-guardrail matcher, where an unrecognized/malformed
+ * glob shape failing towards "still matches" is the safety-correct default (see its own doc comment); adding
+ * `!`-parsing there would mean a maintainer's literal path starting with `!` (rare, but not impossible) is
+ * silently reinterpreted as an exclusion instead of guarding it. This function is opt-in for callers that
+ * explicitly want exclusion semantics (today: the screenshotTableGate `whenPaths` scope check) and leaves
+ * every existing `matchesAny` caller — including hardGuardrailGlobs — byte-identical.
+ *
+ * The INCLUDE half reuses `matchesAny` unchanged, so an over-complex include glob keeps its existing
+ * fail-toward-matching default. The EXCLUDE half deliberately does NOT reuse `matchesAny` for this: failing
+ * an unsafe glob toward "matches" is safe for an include (worst case, more paths are considered in scope) but
+ * WRONG for an exclude (an over-complex exclude glob resolving to "matches everything" would silently widen
+ * what gets excluded, shrinking a safety gate's coverage — the opposite failure direction from what the gate
+ * exists for). The exclude half therefore compiles each glob with `globToRegExp` directly, whose own
+ * NEVER_MATCHES fallback for an unsafe glob is exactly right here: a malformed/pathological exclude excludes
+ * NOTHING rather than excluding everything, so gate coverage can only ever be too WIDE from a bad exclude
+ * glob, never too narrow.
+ */
+export function matchesAnyWithExclusions(path: string, globs: string[]): boolean {
+  const includes: string[] = [];
+  const excludes: string[] = [];
+  for (const glob of globs) {
+    if (glob.startsWith("!") && glob.length > 1) excludes.push(glob.slice(1));
+    else includes.push(glob);
+  }
+  if (includes.length === 0) return false;
+  if (!matchesAny(path, includes)) return false;
+  if (excludes.length === 0) return true;
+  const canonicalPath = canonicalize(path);
+  return !excludes.some((exclude) => globToRegExp(exclude).test(canonicalPath));
+}
+
 /**
  * The changed paths (if any) that trip a hard guardrail. A non-empty result means the PR touches a guarded
  * path and MUST fall through to a human — loopover may neither auto-merge nor auto-close it. Pure.

src/queue-intelligence.ts

@@ -40,7 +40,34 @@ const HIGH_ISSUE_QUALITY_THRESHOLD = 0.7;
 const PENDING_STALE_THRESHOLD_DAYS = 2;
 const MILLISECONDS_PER_DAY = 1000 * 60 * 60 * 24;
 
-export const FORBIDDEN_PUBLIC_COMMENT_WORDS = [
+/**
+ * #9432: ordinary English that carries gittensor meaning ONLY in context. Each of these is a word a perfectly
+ * safe review of this codebase's own gate/scoring code uses naturally -- "updates the ranking comparator",
+ * "this improves reviewability", "the reward path". They are still matched by default (unchanged), but a repo
+ * that has POSITIVELY confirmed via {@link isPublicScoreTermSafeForRepo}'s allowlist that this vocabulary is
+ * safe public language for it can exempt them, exactly as bare "score" already could.
+ *
+ * WHY THIS IS SAFE, and why the split is where it is: the risk this filter exists to stop is a leaked private
+ * VALUE, and a value never appears as a bare noun -- it appears qualified ("reward estimate 12 TAO", "trust
+ * score 0.82", "score preview"). Every one of those QUALIFIED forms lives in
+ * {@link ALWAYS_FORBIDDEN_PUBLIC_COMMENT_WORDS} below and stays enforced for every repo, allowlisted or not.
+ * A bare "reward" or "ranking" with no number attached leaks nothing on its own. This generalizes the exact
+ * judgment `allowBareScoreTerm` already made for "score" to its siblings, rather than inventing a new one.
+ */
+export const AMBIGUOUS_PUBLIC_COMMENT_WORDS = [
+  "rewards",
+  "reward",
+  "farming",
+  "rankings",
+  "ranking",
+  "cohort",
+  "reviewability",
+] as const;
+
+/** Terms that name a private concept outright and are NEVER exemptible, for any repo. A leaked value is
+ *  necessarily qualified (see AMBIGUOUS_PUBLIC_COMMENT_WORDS' rationale), so every qualified form belongs
+ *  here -- this list is what actually holds the public/private boundary. */
+export const ALWAYS_FORBIDDEN_PUBLIC_COMMENT_WORDS = [
   "wallet",
   "hotkey",
   "raw trust score",
@@ -53,12 +80,8 @@ export const FORBIDDEN_PUBLIC_COMMENT_WORDS = [
   "reward estimate",
   "estimated rewards",
   "estimated reward",
-  "rewards",
-  "reward",
-  "farming",
   "private reviewability",
   "reviewability internals",
-  "reviewability",
   "private scoreability",
   "scoreability",
   "score preview",
@@ -67,15 +90,19 @@ export const FORBIDDEN_PUBLIC_COMMENT_WORDS = [
   "score estimate",
   "private rankings",
   "private ranking",
-  "rankings",
-  "ranking",
-  "cohort",
   "miner-originated",
   "miner originated",
   "human-originated",
   "human originated",
 ] as const;
 
+/** Every forbidden term, both tiers -- the default (non-exempt) matching set. Preserved as a single exported
+ *  list so existing consumers and the redaction-parity test keep one canonical vocabulary to compare against. */
+export const FORBIDDEN_PUBLIC_COMMENT_WORDS = [
+  ...ALWAYS_FORBIDDEN_PUBLIC_COMMENT_WORDS,
+  ...AMBIGUOUS_PUBLIC_COMMENT_WORDS,
+] as const;
+
 // A bare "score" is checked separately from the substring list above (not folded in as another entry):
 // FORBIDDEN_PUBLIC_COMMENT_WORDS is matched with a plain case-insensitive `.includes()`, and an unqualified
 // "score" substring also matches ordinary English words that carry no gittensor meaning at all ("underscore",
@@ -218,7 +245,13 @@ export function shouldWarnPublicScoreTermsAllowlistUnset(env: Record<string, str
  *  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) {
+  // #9432: an allowlisted repo exempts the AMBIGUOUS tier (ordinary English -- "reward", "ranking",
+  // "reviewability", ...) along with bare "score"; the ALWAYS_FORBIDDEN tier is checked for every repo,
+  // allowlisted or not, because that is where every qualified private-value form lives. Same flag, same
+  // allowlist, same positively-confirmed-per-repo requirement -- see AMBIGUOUS_PUBLIC_COMMENT_WORDS for why
+  // the split falls where it does.
+  const forbidden = options?.allowBareScoreTerm ? ALWAYS_FORBIDDEN_PUBLIC_COMMENT_WORDS : FORBIDDEN_PUBLIC_COMMENT_WORDS;
+  for (const forbiddenWord of forbidden) {
     if (comment.toLowerCase().includes(forbiddenWord.toLowerCase())) {
       throw new Error(`Public comment contains forbidden word: "${forbiddenWord}"`);
     }

src/selfhost/inert-config.ts

@@ -0,0 +1,109 @@
+// #9433: config-dependent fixes that ship INERT.
+//
+// Several production-behaviour fixes are gated on env vars that were never set, so the shipped code path
+// stayed inert and the deployment silently behaved exactly like the pre-fix build. No unit test can catch this
+// class: tests verify the mechanism works WHEN ENABLED, never that an operator set the variable. The confirmed
+// instance (#9433) — `LOOPOVER_PUBLIC_SCORE_TERMS_ALLOWED_REPOS` unset, so every AI review narrative
+// containing the ordinary word "score" had its whole summary silently replaced by a placeholder — sat live for
+// weeks with a green test suite.
+//
+// WHY THIS IS A REPORT, NOT MORE BOOT WARNINGS. The obvious fix — one `console.error` per var, mirroring
+// `shouldWarnRagEmbedUnavailable` — does not generalize, and following it blindly makes things worse. Verified
+// while writing this: `LOOPOVER_PUBLIC_STATS_REPOS` is unset on the self-host box and that is CORRECT, because
+// the public-stats surface is served by the Cloudflare Worker (where wrangler.jsonc sets it to four repos).
+// A boot warning there would fire on every self-host deployment forever, for a non-problem — the same alert
+// fatigue that let a genuinely broken backup alert be ignored for 8 days. A warning is only justified when
+// "unset" is wrong for EVERY deployment; that is rare, and each such case still earns its own dedicated
+// warning at its own call site (the two that exist today are correct and stay).
+//
+// What is missing is not more noise but ANSWERABILITY: an operator has no way to ask "which config-gated
+// behaviours are currently inert on this box?" without reading boot logs they have long since scrolled past.
+// This module answers exactly that question, on demand, with zero steady-state noise.
+//
+// SCOPE, deliberately narrow. This reports only vars whose unset state changes OUTPUT CORRECTNESS — review
+// content, gate disposition, or a published number. It does NOT report the ~100 `Default OFF` convergence
+// flags in env.d.ts: those are opt-in features whose absence keeps the review path byte-identical, so listing
+// them would bury the few entries that matter under a wall of working-as-intended noise.
+
+/** One config-gated behaviour that is currently inert. */
+export type InertConfigEntry = {
+  /** The env var an operator would set. */
+  key: string;
+  /** What stops working while it is unset — phrased as the OBSERVABLE effect, not the mechanism. */
+  impact: string;
+  /**
+   * Whether an unset value is wrong for every deployment, or legitimately correct for some.
+   *
+   * `always-wrong` earns a boot warning too (and today's two both have one). `deployment-specific` must NOT
+   * be warned about — it is exactly the `LOOPOVER_PUBLIC_STATS_REPOS` case above, where the same unset value
+   * is correct on one runtime and a defect on another, and only the operator knows which they are running.
+   */
+  severity: "always-wrong" | "deployment-specific";
+};
+
+/** Reads only the vars it names, so it is safe to call with `process.env` directly. */
+export type InertConfigEnv = Record<string, string | undefined>;
+
+function unset(value: string | undefined): boolean {
+  return (value ?? "").trim() === "";
+}
+
+/**
+ * Every config-gated behaviour currently inert in `env`, in a stable order.
+ *
+ * PURE — no IO, no clock, no logging — so the `/metrics` gauge, a `/ready` field, and a test can all read the
+ * identical answer rather than three hand-maintained lists drifting apart (which is the same drift class this
+ * whole issue is about).
+ */
+export function inertConfigEntries(env: InertConfigEnv): InertConfigEntry[] {
+  const entries: InertConfigEntry[] = [];
+
+  // The confirmed #9433 instance. Fail-closed by design, and the fail-closed direction is content-destroying:
+  // sanitizePublicComment THROWS on a bare "score" match and the caller degrades to a generic placeholder, so
+  // an unset allowlist silently strips narrative sentences on every repo. Correct for a deployment whose repos
+  // genuinely carry private trust/reward data, hence deployment-specific rather than always-wrong.
+  if (unset(env.LOOPOVER_PUBLIC_SCORE_TERMS_ALLOWED_REPOS)) {
+    entries.push({
+      key: "LOOPOVER_PUBLIC_SCORE_TERMS_ALLOWED_REPOS",
+      impact:
+        "AI review narratives lose any sentence using ordinary scoring vocabulary (\"score\", \"ranking\", \"reward\", \"reviewability\"), silently — the published summary looks fine, just shorter.",
+      severity: "deployment-specific",
+    });
+  }
+
+  // Verified unset on the ORB self-host box and CORRECT there: the public-stats surface runs on the Cloudflare
+  // Worker, whose wrangler.jsonc sets it. Reported (never warned) precisely so an operator on a runtime that
+  // DOES serve /v1/public/stats can see that the own-ledger half is publishing zeros.
+  if (unset(env.LOOPOVER_PUBLIC_STATS_REPOS)) {
+    entries.push({
+      key: "LOOPOVER_PUBLIC_STATS_REPOS",
+      impact:
+        "The own-ledger half of /v1/public/stats reports zero (disposition counts, reversal-grounded accuracy, weekly totals). Harmless on a runtime that does not serve public stats; silently wrong on one that does.",
+      severity: "deployment-specific",
+    });
+  }
+
+  // Redaction is flag-gated while DETECTION is not (verified: reviewInputHasPromptInjection and its hold run
+  // unconditionally), so an unset flag never lets a manipulated verdict through — it only means the reviewer
+  // sees the raw injected text rather than a defanged copy. Reported because the inconclusive-finding copy
+  // tells a reader the content "was redacted before review", which is untrue while this is off.
+  if (unset(env.LOOPOVER_REVIEW_SAFETY)) {
+    entries.push({
+      key: "LOOPOVER_REVIEW_SAFETY",
+      impact:
+        "Prompt-injection text is still DETECTED and still holds the PR, but is not defanged before the model sees it — and the public finding claims it was redacted.",
+      severity: "deployment-specific",
+    });
+  }
+
+  return entries;
+}
+
+/** Stable, bounded label values for the `/metrics` gauge — the key set is fixed in code, so cardinality is
+ *  bounded by construction and an operator can alert on a specific key without a cardinality risk. */
+export function inertConfigGaugeSamples(env: InertConfigEnv): Array<{ labels: Record<string, string>; value: number }> {
+  return inertConfigEntries(env).map((entry) => ({
+    labels: { key: entry.key, severity: entry.severity },
+    value: 1,
+  }));
+}

src/selfhost/metrics.ts

@@ -57,6 +57,7 @@ export const DEFAULT_METRIC_META: readonly (readonly [string, MetricMeta])[] = [
   ["loopover_backup_acknowledged", { help: "1 when SQLite backup is acknowledged or Postgres is in use; 0 when the boot backup advisory would fire.", type: "gauge" }],
   ["loopover_config_dir_empty_acknowledged", { help: "1 when LOOPOVER_REPO_CONFIG_DIR is unset, has entries, or is acknowledged; 0 when it's configured but the mounted directory is empty.", type: "gauge" }],
   ["loopover_http_requests_total", { help: "HTTP app requests by response status class.", type: "counter" }],
+  ["loopover_inert_config", { help: "One series per config-gated behaviour currently INERT on this instance (#9433) — labelled by env var key and whether an unset value is always wrong or deployment-specific. No series ⇒ nothing inert. See src/selfhost/inert-config.ts.", type: "gauge" }],
   ["loopover_http_request_duration_seconds", { help: "HTTP app request duration in seconds, labelled by bounded route group (see httpRouteGroup).", type: "histogram" }],
   ["loopover_visual_capture_total", { help: "Visual capture attempts by result -- a browserless outage is otherwise invisible while it silently degrades screenshots to dash cells, and the screenshot gate treats absent evidence as a close signal (#9487).", type: "counter" }],
   ["loopover_webhook_dedup_total", { help: "Webhook deliveries deduplicated before enqueue.", type: "counter" }],