refactor(review): unify six advisory-feature activation checks behind one shared resolver

.gittensory.yml.example

@@ -1106,24 +1106,28 @@ settings:
 #     auto_pause_after_reviewed_commits: 3
 
 # Per-repo activation overrides for the converged review features that ship behind a deployment-wide
-# GITTENSORY_REVIEW_* env kill-switch (rag/reputation/unifiedComment/safety/grounding/e2eTests/improvementSignal).
-# Each key is `true` (force on for this repo, subject to the env flag still being enabled), `false` (force off),
-# or omitted (falls back to the GITTENSORY_REVIEW_REPOS allowlist default -- an operator who sets nothing keeps
-# today's behavior). Exception: `safety` is force-on-only -- an untrusted repo-controlled `false` is treated as
-# "no opinion" rather than an active force-off (#2269), since a lower-trust actor must never be able to silently
-# defeat the operator's own security-hardening enablement. `e2eTests` (#4190) and `improvementSignal` (#4738,
-# foundation phase of the #4737 PR-improvement-signal epic) are plain symmetric overrides like
-# rag/reputation/unifiedComment -- neither AI-generated test content nor a read-only advisory quality-delta
-# signal carries a security-hardening rationale to protect from a repo-controlled override, so there's no
-# force-on/force-off asymmetry for either. `improvementSignal` is activation wiring only for now -- no tier
-# reads the resolved value yet.
+# GITTENSORY_REVIEW_* env kill-switch (rag/reputation/unifiedComment/safety/grounding/e2eTests/screenshots/
+# improvementSignal). Each key is `true` (force on for this repo, subject to the env flag still being
+# enabled), `false` (force off), or omitted (falls back to the GITTENSORY_REVIEW_REPOS allowlist default --
+# an operator who sets nothing keeps today's behavior). Exception: `safety` is force-on-only -- an untrusted
+# repo-controlled `false` is treated as "no opinion" rather than an active force-off (#2269), since a
+# lower-trust actor must never be able to silently defeat the operator's own security-hardening enablement.
+# `e2eTests` (#4190), `screenshots` (#4616), and `improvementSignal` (#4738, foundation phase of the #4737
+# PR-improvement-signal epic) are plain symmetric overrides like rag/reputation/unifiedComment -- none carries
+# a security-hardening or full-file-fetch rationale (unlike safety/grounding) that would justify a force-on/
+# force-off asymmetry. `screenshots` sits UNDER the separate, richer `review.visual.*` block further below
+# (route/preview-URL config, and `review.visual.enabled: false` as an always-available additional force-off);
+# this key only answers "does capture run for this repo at all," the same question its siblings answer for
+# their own feature. `improvementSignal` is activation wiring only for now -- no tier reads the resolved
+# value yet.
 # features:
 #   rag: true
 #   reputation: false
 #   unifiedComment: true
 #   safety: true
 #   grounding: true
 #   e2eTests: false
+#   screenshots: false
 #   improvementSignal: false
 
 # Registry-review lane (#2435): lets a self-hosted maintainer point gittensory at their OWN structured

config/examples/gittensory.full.yml

@@ -1119,24 +1119,28 @@ settings:
 #     auto_pause_after_reviewed_commits: 3
 
 # Per-repo activation overrides for the converged review features that ship behind a deployment-wide
-# GITTENSORY_REVIEW_* env kill-switch (rag/reputation/unifiedComment/safety/grounding/e2eTests/improvementSignal).
-# Each key is `true` (force on for this repo, subject to the env flag still being enabled), `false` (force off),
-# or omitted (falls back to the GITTENSORY_REVIEW_REPOS allowlist default -- an operator who sets nothing keeps
-# today's behavior). Exception: `safety` is force-on-only -- an untrusted repo-controlled `false` is treated as
-# "no opinion" rather than an active force-off (#2269), since a lower-trust actor must never be able to silently
-# defeat the operator's own security-hardening enablement. `e2eTests` (#4190) and `improvementSignal` (#4738,
-# foundation phase of the #4737 PR-improvement-signal epic) are plain symmetric overrides like
-# rag/reputation/unifiedComment -- neither AI-generated test content nor a read-only advisory quality-delta
-# signal carries a security-hardening rationale to protect from a repo-controlled override, so there's no
-# force-on/force-off asymmetry for either. `improvementSignal` is activation wiring only for now -- no tier
-# reads the resolved value yet.
+# GITTENSORY_REVIEW_* env kill-switch (rag/reputation/unifiedComment/safety/grounding/e2eTests/screenshots/
+# improvementSignal). Each key is `true` (force on for this repo, subject to the env flag still being
+# enabled), `false` (force off), or omitted (falls back to the GITTENSORY_REVIEW_REPOS allowlist default --
+# an operator who sets nothing keeps today's behavior). Exception: `safety` is force-on-only -- an untrusted
+# repo-controlled `false` is treated as "no opinion" rather than an active force-off (#2269), since a
+# lower-trust actor must never be able to silently defeat the operator's own security-hardening enablement.
+# `e2eTests` (#4190), `screenshots` (#4616), and `improvementSignal` (#4738, foundation phase of the #4737
+# PR-improvement-signal epic) are plain symmetric overrides like rag/reputation/unifiedComment -- none carries
+# a security-hardening or full-file-fetch rationale (unlike safety/grounding) that would justify a force-on/
+# force-off asymmetry. `screenshots` sits UNDER the separate, richer `review.visual.*` block further below
+# (route/preview-URL config, and `review.visual.enabled: false` as an always-available additional force-off);
+# this key only answers "does capture run for this repo at all," the same question its siblings answer for
+# their own feature. `improvementSignal` is activation wiring only for now -- no tier reads the resolved
+# value yet.
 # features:
 #   rag: true
 #   reputation: false
 #   unifiedComment: true
 #   safety: true
 #   grounding: true
 #   e2eTests: false
+#   screenshots: false
 #   improvementSignal: false
 
 # Registry-review lane (#2435): lets a self-hosted maintainer point gittensory at their OWN structured

packages/gittensory-engine/src/focus-manifest.ts

@@ -208,22 +208,34 @@ export type CopycatGateMode = "off" | "warn" | "label" | "block";
 // (#4100) is now migrated too — its original "coupled to the merge/close DISPOSITION path" blocker was the
 // removed AI CI-refutation path (grounding-wire.ts's aiCiRefutationActive is now a vestigial historical-
 // compatibility helper with zero real callers); grounding today only shapes reviewer PROMPT content, same
-// shape as rag/reputation. `screenshots` has its own richer `visual:` block instead (review.visual.enabled,
-// #4083) since it carries more than a single boolean. contentLane got its own richer `contentLane:` block below
-// (#2435) instead of a boolean here, since it resolves to a whole RegistryLaneSpec, not an on/off toggle — see
-// resolveRegistryLaneSpec in review/content-lane/spec-resolver.ts. `selftune` (#4104) ALSO deliberately lives
-// outside this block, as its own top-level `review.selftune` field below — it has no `GITTENSORY_REVIEW_REPOS`
-// allowlist to fall back to (its own repo scoping is `isAgentConfigured`, a different consent boundary), so it
-// doesn't fit this resolver's env-kill-switch → override → allowlist-default shape; see `selfTuneRepos` in
-// `review/selftune-wire.ts`. `e2eTests` (#4190, part of the #4189 E2E-test-generation epic) fits this shape
-// exactly as a plain symmetric override — unlike `safety`/`grounding` it has no force-on-only or force-off-only
-// floor/ceiling, since AI-generated test content carries no security-hardening or full-file-fetch rationale to
-// protect from a repo-controlled override. `improvementSignal` (#4738, foundation phase of the #4737 PR-
-// improvement-signal epic) is likewise a plain symmetric override: it is a READ-ONLY advisory quality-delta
-// signal, not a security control, so a repo-level `false` behaves like any other plain override with no
-// floor/ceiling. This is activation wiring only -- no tier reads the resolved value yet (sibling sub-issues
-// #4739-#4746 build the deterministic/LLM/panel behavior that will gate on it).
-export const CONVERGED_FEATURE_KEYS = ["rag", "reputation", "unifiedComment", "safety", "grounding", "e2eTests", "improvementSignal"] as const;
+// shape as rag/reputation. contentLane got its own richer `contentLane:` block below (#2435) instead of a
+// boolean here, since it resolves to a whole RegistryLaneSpec, not an on/off toggle — see
+// resolveRegistryLaneSpec in review/content-lane/spec-resolver.ts (its own precedence already matches this
+// block's env-kill-switch → override → allowlist-default shape one-for-one; it just isn't literally routed
+// through resolveConvergedFeature yet — a disclosed, low-priority fast-follow, #4616). `selftune` (#4104)
+// ALSO deliberately lives outside this block, as its own top-level `review.selftune` field below — it has no
+// `GITTENSORY_REVIEW_REPOS` allowlist to fall back to (its own repo scoping is `isAgentConfigured`, a
+// different consent boundary), so it doesn't fit this resolver's env-kill-switch → override → allowlist-
+// default shape; see `selfTuneRepos` in `review/selftune-wire.ts`. `e2eTests` (#4190, part of the #4189
+// E2E-test-generation epic) fits this shape exactly as a plain symmetric override — unlike `safety`/
+// `grounding` it has no force-on-only or force-off-only floor/ceiling, since AI-generated test content
+// carries no security-hardening or full-file-fetch rationale to protect from a repo-controlled override.
+// `screenshots` (#4616) joined this block for the SAME reason `e2eTests` fits it plainly: capturing a
+// before/after render of the PR's own web-visible files carries no security-hardening or full-file-fetch
+// rationale either, so it gets the standard override, not an asymmetric one. Before #4616 it had NO
+// `features:` override at all (env flag AND allowlist only) despite being documented right next to its six
+// siblings in `.gittensory.yml.example` — a self-hoster who guessed `features.screenshots: true` (a natural
+// guess given the sibling keys) found it silently did nothing. `features.screenshots` is layered UNDER the
+// separate, richer `review.visual.*` block (route/preview-URL config, #3609/#3610, and `review.visual.enabled:
+// false` as an always-available additional force-off, #4083) — that block still narrows/disables capture
+// AFTER this key decides whether capture is even attempted for the repo at all; the two are independent and
+// `review.visual.enabled` keeps its own existing force-off-only semantics untouched by this change.
+// `improvementSignal` (#4738, foundation phase of the #4737 PR-improvement-signal epic) is likewise a plain
+// symmetric override: it is a READ-ONLY advisory quality-delta signal, not a security control, so a repo-
+// level `false` behaves like any other plain override with no floor/ceiling. This is activation wiring only
+// -- no tier reads the resolved value yet (sibling sub-issues #4739-#4746 build the deterministic/LLM/panel
+// behavior that will gate on it).
+export const CONVERGED_FEATURE_KEYS = ["rag", "reputation", "unifiedComment", "safety", "grounding", "e2eTests", "screenshots", "improvementSignal"] as const;
 export type ConvergedFeatureKey = (typeof CONVERGED_FEATURE_KEYS)[number];
 
 /** Per-repo activation overrides for the converged review features (`features:` block). `true`/`false` force the
@@ -967,6 +979,7 @@ const EMPTY_FEATURES_CONFIG: FocusManifestFeaturesConfig = {
   safety: null,
   grounding: null,
   e2eTests: null,
+  screenshots: null,
   improvementSignal: null,
 };
 

src/queue/processors.ts

@@ -385,7 +385,6 @@ import {
 // unchanged -- those tests are deeply interspersed with unrelated ones in that file, not in a cleanly
 // extractable describe block, so relocating them is deliberately deferred rather than forced into this PR.
 export { claimPrActuationLock, releasePrActuationLock } from "./transient-locks";
-import { screenshotsAllowed } from "../review/visual-wire";
 import { isVisualPath } from "../review/visual/paths";
 import { buildCapture, fetchShotContentBlock, hasSuccessfulBotCapture, resolveVisualRoutes, type CaptureRoute } from "../review/visual/capture";
 import {
@@ -515,7 +514,7 @@ import { resolveE2eTestGenInstructions, runGittensoryE2eTestGeneration } from ".
 import { commitE2eTestToPrBranch } from "../github/e2e-test-commit";
 import {
   buildRepoCultureProfileContext,
-  isRepoCultureProfileEnabled,
+  shouldApplyRepoCultureProfile,
 } from "../review/repo-culture-profile-wire";
 import { applyReviewMemorySuppression, getCachedReviewSuppressions, invalidateReviewSuppressionCache, shouldApplyReviewMemory } from "../review/review-memory-wire";
 import {
@@ -7837,10 +7836,9 @@ export async function runAiReviewForAdvisory(
     // (typical PR size, common accepted labels) and appends it as additive grounding — exactly like RAG. Both
     // gates OFF (default) → NO new branch: no D1 read, and `cultureProfileContext` is left undefined so the
     // prompt is byte-identical to today. Fully fail-safe (any error/insufficient-history degrades to "").
-    const cultureProfileContext =
-      isRepoCultureProfileEnabled(env) && args.reviewCultureProfile === true
-        ? await buildRepoCultureProfileContext(env, args.repoFullName)
-        : undefined;
+    const cultureProfileContext = shouldApplyRepoCultureProfile(env, args.reviewCultureProfile === true)
+      ? await buildRepoCultureProfileContext(env, args.repoFullName)
+      : undefined;
     // Review-enrichment (#1472, flag-gated by GITTENSORY_REVIEW_ENRICHMENT + REES_URL). POST the PR to the external
     // REES for the heavy/external analysis the reviewer can't run (dependency CVEs, secrets, license/EOL/supply-chain);
     // its public-safe brief splices into the prompt next to grounding + RAG. Flag-OFF (default) → no call, no branch,
@@ -10668,7 +10666,7 @@ async function maybePublishPrPublicSurface(
             // invalidation) can refresh independently of this PR's head SHA, exactly like RAG's vector index —
             // so a repo with it active also bypasses the AI-review result cache rather than fingerprinting a
             // value that can't prove freshness.
-            cultureProfile: isRepoCultureProfileEnabled(env) && reviewCultureProfile === true,
+            cultureProfile: shouldApplyRepoCultureProfile(env, reviewCultureProfile === true),
             // Impact map (#2182-#2186): queries the SAME live vector index RAG does (computeImpactMap issues
             // its own retrieveContextWithMetrics calls), so it can go stale for the SAME head SHA exactly like
             // RAG — a repo with it active also bypasses the AI-review result cache.
@@ -11652,17 +11650,19 @@ async function maybePublishPrPublicSurface(
         gate: commentGate,
         duplicateWinnerEnabled,
       });
-      // Visual before/after capture (visual-capture port). Fires ONLY when (1) the global flag + per-repo
-      // cutover gate both allow it (screenshotsAllowed) AND (2) the PR touches WEB-VISIBLE files (isVisualPath
-      // — frontend pages / public OG images; backend .ts/.md/.json PRs never qualify). Fully wrapped in
+      // Visual before/after capture (visual-capture port). Fires ONLY when (1) the "screenshots" converged
+      // feature resolves active for this repo (resolveConvergedFeature — the global flag AND (a per-repo
+      // `features.screenshots` override OR the cutover allowlist default), #4616; reuses the manifest this
+      // pass already loaded above, no extra fetch) AND (2) the PR touches WEB-VISIBLE files (isVisualPath —
+      // frontend pages / public OG images; backend .ts/.md/.json PRs never qualify). Fully wrapped in
       // try/catch + defaults to [] so a capture failure (render timeout, missing binding, GitHub hiccup) can
       // NEVER sink the review — it just omits the "Visual preview" section. Flag-OFF (default) ⇒ this block is
       // skipped entirely and the unified comment is byte-identical.
       let beforeAfter: CaptureRoute[] = [];
       const visualFiles = unifiedFiles
         .map((file) => file.path)
         .filter(isVisualPath);
-      if (screenshotsAllowed(env, repoFullName) && visualFiles.length > 0) {
+      if (resolveConvergedFeature(env, repoFocusManifestForComment, "screenshots", repoFullName) && visualFiles.length > 0) {
         try {
           const token = await createInstallationToken(env, installationId);
           // review.visual (#3609 / #3610): an explicit per-repo preview-URL template / route list. Absent config