fix(review): keep grounding bound to repo allowlist

src/review/feature-activation.ts

@@ -6,11 +6,13 @@
 // (`features:` block). The precedence, highest to lowest:
 //   1. GLOBAL env flag (GITTENSORY_REVIEW_*) — a MASTER KILL-SWITCH. Off ⇒ the feature never runs anywhere,
 //      regardless of any per-repo override (so an operator keeps one deploy-wide off switch per feature).
-//   2. Per-repo `features:` override — `true`/`false` forces the feature on/off for this repo. EXCEPTION:
+//   2. Per-repo `features:` override — `true`/`false` forces the feature on/off for this repo. EXCEPTIONS:
 //      `safety` (prompt-injection defanging) is security-critical and `.gittensory.yml` lives in the repo
 //      itself, writable by a lower-trust actor than the operator — so a repo override may only TIGHTEN
 //      (force-on) the operator's global enablement, never loosen it. `features.safety: false` is treated as
 //      "no opinion" (falls through to the allowlist default below) rather than an active force-off (#2269).
+//      `grounding` can fetch full post-change file contents for the AI prompt, so the operator allowlist remains
+//      mandatory; a repo override may only disable grounding inside that allowlist.
 //   3. `GITTENSORY_REVIEW_REPOS` allowlist — the back-compat DEFAULT when the manifest says nothing, so a repo
 //      that sets no `features:` block behaves exactly as it did before this change.
 //
@@ -38,7 +40,8 @@ const FEATURE_GLOBAL_FLAG: Record<ConvergedFeatureKey, (env: Env) => boolean> =
  * Resolve whether a converged feature is active for a repo, given the already-loaded manifest (or null). Pure +
  * synchronous so it carries no I/O and is the single unit-tested place the precedence lives. Precedence: env
  * kill-switch (off ⇒ false) → per-repo `features:` override → `GITTENSORY_REVIEW_REPOS` allowlist default.
- * `safety` is asymmetric: an override can only force it ON, never force it OFF (#2269) — see the file header.
+ * `safety` is asymmetric: an override can only force it ON, never force it OFF (#2269). `grounding` is also
+ * asymmetric in the opposite direction: a repo override can only force it OFF, never bypass the operator allowlist.
  */
 export function resolveConvergedFeature(
   env: Env,
@@ -50,9 +53,11 @@ export function resolveConvergedFeature(
   const override = manifest?.features?.[feature] ?? null;
   // Security-critical: a repo-controlled override must not silently defeat the operator's global enablement.
   // `false` is downgraded to "no opinion" so it falls through to the allowlist default instead of forcing off.
-  if (feature === "safety") return override === true || isConvergenceRepoAllowed(env, repoFullName);
+  const allowlisted = isConvergenceRepoAllowed(env, repoFullName);
+  if (feature === "safety") return override === true || allowlisted;
+  if (feature === "grounding") return allowlisted && override !== false;
   if (override !== null) return override; // explicit per-repo on/off
-  return isConvergenceRepoAllowed(env, repoFullName); // back-compat allowlist default
+  return allowlisted; // back-compat allowlist default
 }
 
 /**

test/unit/feature-activation.test.ts

@@ -80,6 +80,20 @@ describe("resolveConvergedFeature — safety is force-on-only, never force-off (
   });
 });
 
+describe("resolveConvergedFeature — grounding remains allowlist-bound", () => {
+  it("does not let a repo manifest force grounding ON outside the operator allowlist", () => {
+    const e = env({ GITTENSORY_REVIEW_GROUNDING: "true", GITTENSORY_REVIEW_REPOS: "other/repo" });
+    expect(resolveConvergedFeature(e, manifestWith({ grounding: true }), "grounding", REPO)).toBe(false);
+  });
+
+  it("allows an allowlisted repo to enable grounding by default and force it OFF per repo", () => {
+    const e = env({ GITTENSORY_REVIEW_GROUNDING: "true", GITTENSORY_REVIEW_REPOS: REPO });
+    expect(resolveConvergedFeature(e, manifestWith({}), "grounding", REPO)).toBe(true);
+    expect(resolveConvergedFeature(e, manifestWith({ grounding: true }), "grounding", REPO)).toBe(true);
+    expect(resolveConvergedFeature(e, manifestWith({ grounding: false }), "grounding", REPO)).toBe(false);
+  });
+});
+
 describe("convergedFeatureActive — async (loads the cached manifest)", () => {
   it("short-circuits to false WITHOUT loading the manifest when the env flag is off", async () => {
     // DB-less env: if it tried to load the manifest it would throw; returning false proves the short-circuit.