Merge branch 'main' into fix/hotkey-coldkey-secret-scan-false-positive

Author: loopover-orb[bot]

apps/loopover-ui/src/lib/config-generator-yaml.test.ts

@@ -96,4 +96,71 @@ describe("formStateToYaml", () => {
       [HEADER, "gate:", "  aiReview:", `    model: ${JSON.stringify(model)}`].join("\n"),
     );
   });
+
+  // Regression (#8022): rees.analyzers overrides never reached the preview at all, since this module
+  // declared its own narrower GeneratorFormState duplicate with no `rees` field.
+  describe("review.enrichment (REES analyzer overrides, #8022)", () => {
+    it("serializes rees.analyzers overrides as a review.enrichment map", () => {
+      const state: GeneratorFormState = {
+        rees: { analyzers: { "deep-nesting": true, "error-swallow": false } },
+      };
+      expect(formStateToYaml(state)).toBe(
+        [
+          HEADER,
+          "review:",
+          "  enrichment:",
+          "    deep-nesting: true",
+          "    error-swallow: false",
+        ].join("\n"),
+      );
+    });
+
+    it("sorts analyzer entries alphabetically regardless of insertion order", () => {
+      const state: GeneratorFormState = {
+        rees: { analyzers: { "zebra-analyzer": true, "alpha-analyzer": true } },
+      };
+      expect(formStateToYaml(state)).toBe(
+        [
+          HEADER,
+          "review:",
+          "  enrichment:",
+          "    alpha-analyzer: true",
+          "    zebra-analyzer: true",
+        ].join("\n"),
+      );
+    });
+
+    it("omits the review.enrichment block when rees.analyzers is empty", () => {
+      expect(formStateToYaml({ rees: { analyzers: {} } })).toBe(HEADER);
+    });
+
+    it("omits the review.enrichment block when rees is entirely unset", () => {
+      expect(formStateToYaml({ rees: undefined })).toBe(HEADER);
+    });
+
+    it("keeps a false override in the output rather than treating it as unset", () => {
+      const state: GeneratorFormState = { rees: { analyzers: { "secret-scan": false } } };
+      expect(formStateToYaml(state)).toBe(
+        [HEADER, "review:", "  enrichment:", "    secret-scan: false"].join("\n"),
+      );
+    });
+
+    it("renders both gate.aiReview and review.enrichment when both are configured", () => {
+      const state: GeneratorFormState = {
+        gate: { aiReview: { provider: "anthropic" } },
+        rees: { analyzers: { "deep-nesting": true } },
+      };
+      expect(formStateToYaml(state)).toBe(
+        [
+          HEADER,
+          "gate:",
+          "  aiReview:",
+          "    provider: anthropic",
+          "review:",
+          "  enrichment:",
+          "    deep-nesting: true",
+        ].join("\n"),
+      );
+    });
+  });
 });

apps/loopover-ui/src/lib/config-generator-yaml.ts

@@ -2,25 +2,23 @@
 // text builder — the reviewable-output half of the config generator; field-group panels (#2208 and
 // siblings) own collecting GeneratorFormState, this module only turns it into text. Key names and
 // nesting match the real manifest schema (packages/loopover-engine/src/focus-manifest.ts's
-// gateConfigToJson `gate.aiReview` shape, the same shape documented in this repo's own root
-// .loopover.yml under the commented-out `aiReview:` example) — no parallel schema. Every field is
-// optional so a fresh/partial form never produces invalid output; unset keys are omitted entirely.
-// The generated header names the `.loopover.yml` filename.
-
-export type AiCombineStrategy = "single" | "consensus" | "synthesis";
-export type AiProvider = "anthropic" | "openai";
-
-export type GeneratorGateAiReviewState = {
-  combine?: AiCombineStrategy | null;
-  provider?: AiProvider | null;
-  model?: string | null;
-};
-
-export type GeneratorFormState = {
-  gate?: {
-    aiReview?: GeneratorGateAiReviewState;
-  };
-};
+// gateConfigToJson `gate.aiReview` shape and `review.enrichment` shape, both documented in this repo's
+// own root .loopover.yml.example) — no parallel schema. Every field is optional so a fresh/partial form
+// never produces invalid output; unset keys are omitted entirely. The generated header names the
+// `.loopover.yml` filename.
+//
+// GeneratorFormState is re-exported from config-generator-form-state.ts (#8022) rather than declared a
+// second time here -- ConfigGeneratorPanel collects the one richer state object field-group panels
+// (including ReesAnalyzerFieldGroup) write to, and this module needs the same shape, not a structurally-
+// compatible-but-narrower duplicate that silently can't see fields like `rees`.
+import type { GeneratorFormState } from "./config-generator-form-state";
+
+export type {
+  AiCombineStrategy,
+  AiProvider,
+  GeneratorFormState,
+  GeneratorGateAiReviewState,
+} from "./config-generator-form-state";
 
 const YAML_HEADER = "# .loopover.yml — generated by the config generator";
 
@@ -39,18 +37,39 @@ function yamlLine(indent: number, key: string, value: string): string {
   return `${"  ".repeat(indent)}${key}: ${yamlScalar(value)}`;
 }
 
-/** Serializes the `gate.aiReview` slice of GeneratorFormState into `.loopover.yml` text, omitting
- *  every unset field/block so the preview is always valid YAML (including the empty-state case). */
+// Analyzer names come from the app's own REES_ANALYZERS catalog (validated GeneratorReesState keys), never
+// free text, so a boolean value needs no scalar-quoting -- `true`/`false` are unambiguous YAML literals.
+function yamlBooleanLine(indent: number, key: string, value: boolean): string {
+  return `${"  ".repeat(indent)}${key}: ${value}`;
+}
+
+/** Serializes the `gate.aiReview` and `review.enrichment` slices of GeneratorFormState into `.loopover.yml`
+ *  text, omitting every unset field/block so the preview is always valid YAML (including the empty-state
+ *  case). `review.enrichment` mirrors `state.rees.analyzers` directly (an overrides-only map, matching the
+ *  real manifest field's own shape) rather than going through `reesAnalyzersManifestPatch`'s exact-list
+ *  output, which serializes the REES_ANALYZERS env var, a different representation of the same overrides
+ *  for a different consumer. */
 export function formStateToYaml(state: GeneratorFormState): string {
   const aiReview = state.gate?.aiReview;
   const aiReviewLines: string[] = [];
   if (aiReview?.combine) aiReviewLines.push(yamlLine(2, "combine", aiReview.combine));
   if (aiReview?.provider) aiReviewLines.push(yamlLine(2, "provider", aiReview.provider));
   if (aiReview?.model) aiReviewLines.push(yamlLine(2, "model", aiReview.model));
 
+  const enrichmentEntries = Object.entries(state.rees?.analyzers ?? {}).sort(([a], [b]) =>
+    a.localeCompare(b),
+  );
+
   const lines = [YAML_HEADER];
   if (aiReviewLines.length > 0) {
     lines.push("gate:", "  aiReview:", ...aiReviewLines);
   }
+  if (enrichmentEntries.length > 0) {
+    lines.push(
+      "review:",
+      "  enrichment:",
+      ...enrichmentEntries.map(([name, enabled]) => yamlBooleanLine(2, name, enabled)),
+    );
+  }
   return lines.join("\n");
 }

control-plane/src/tenant-provisioning-driver.ts

@@ -92,16 +92,24 @@ export type FakeDriverCall = {
 
 /** A fake `TenantProvisioningDriver` plus the recorded state a test inspects. */
 export type FakeTenantProvisioningDriver = TenantProvisioningDriver & {
-  /** Tenant names whose container currently "exists" (an in-memory stand-in for real infrastructure). */
+  /** Product-scoped keys (`${product}:${tenant.name}`, same as container-driver.ts's `instanceNameFor`)
+   *  whose container currently "exists" (an in-memory stand-in for real infrastructure). */
   readonly containers: ReadonlySet<string>;
-  /** Tenant names whose database currently "exists". */
+  /** Product-scoped keys whose database currently "exists". */
   readonly databases: ReadonlySet<string>;
-  /** Tenant names whose secrets are currently injected. */
+  /** Product-scoped keys whose secrets are currently injected. */
   readonly injectedSecrets: ReadonlySet<string>;
   /** Every driver step this fake has run, in call order. */
   readonly calls: readonly FakeDriverCall[];
 };
 
+/** Same composite key as container-driver.ts's `instanceNameFor` (#8025) — ORB and AMS tenants that share a
+ *  name must not collide in the fake's in-memory maps (production composes this fake for any step without a
+ *  real backend yet). */
+function instanceKeyFor(request: TenantProvisioningRequest): string {
+  return `${request.product}:${request.tenant.name}`;
+}
+
 /** Minimal in-memory fake for orchestration/contract tests — three in-memory maps stand in for real infra
  *  ("a container exists" / "a DB exists" / "secrets injected"), toggled by the create/destroy steps, plus an
  *  ordered call log. NO Cloudflare, Postgres, or secret-broker IO of any kind. Mirrors
@@ -135,11 +143,11 @@ export function createFakeTenantProvisioningDriver(): FakeTenantProvisioningDriv
     },
     async createContainer(request) {
       record("createContainer", request);
-      containers.add(request.tenant.name);
+      containers.add(instanceKeyFor(request));
     },
     async provisionDatabase(request) {
       record("provisionDatabase", request);
-      databases.add(request.tenant.name);
+      databases.add(instanceKeyFor(request));
       // Deterministic per-tenant fake connection details -- no real IO, no state beyond the existing
       // `databases` set, just enough shape for callers/tests exercising the widened (#7653) return contract.
       const host = `fake-${request.tenant.name}.control-plane.invalid`;
@@ -151,30 +159,33 @@ export function createFakeTenantProvisioningDriver(): FakeTenantProvisioningDriv
     },
     async injectSecrets(request) {
       record("injectSecrets", request);
-      injectedSecrets.add(request.tenant.name);
+      injectedSecrets.add(instanceKeyFor(request));
     },
     async destroyContainer(request) {
       record("destroyContainer", request);
       // Idempotent teardown: the else-branch (nothing to remove) is the "destroy-of-a-nonexistent-tenant"
       // lifecycle path — a no-op, never a throw.
-      if (containers.has(request.tenant.name)) {
-        containers.delete(request.tenant.name);
+      const key = instanceKeyFor(request);
+      if (containers.has(key)) {
+        containers.delete(key);
       }
     },
     async dropDatabase(request) {
       record("dropDatabase", request);
-      if (databases.has(request.tenant.name)) {
-        databases.delete(request.tenant.name);
+      const key = instanceKeyFor(request);
+      if (databases.has(key)) {
+        databases.delete(key);
       }
     },
     async revokeSecrets(request) {
       record("revokeSecrets", request);
-      if (injectedSecrets.has(request.tenant.name)) {
-        injectedSecrets.delete(request.tenant.name);
+      const key = instanceKeyFor(request);
+      if (injectedSecrets.has(key)) {
+        injectedSecrets.delete(key);
       }
     },
     async containerExists(request) {
-      return containers.has(request.tenant.name);
+      return containers.has(instanceKeyFor(request));
     },
   };
 }

control-plane/test/driver-factory.test.ts

@@ -69,21 +69,21 @@ test("withRealDatabaseDriver: overrides provisionDatabase/dropDatabase, forwards
   assert.deepEqual(calls, ["real-provision"]);
   // The fake's own provisionDatabase never ran -- its `databases` set stays empty even though the composed
   // driver's provisionDatabase resolved successfully.
-  assert.equal(base.databases.has("acme"), false);
+  assert.equal(base.databases.has("orb:acme"), false);
 
   await composed.dropDatabase(REQUEST);
   assert.deepEqual(calls, ["real-provision", "real-drop"]);
 
   // Every non-database step still runs against `base` exactly as before composition.
   await composed.createContainer(REQUEST);
-  assert.ok(base.containers.has("acme"));
+  assert.ok(base.containers.has("orb:acme"));
   assert.equal(await composed.containerExists(REQUEST), true);
   await composed.injectSecrets(REQUEST);
-  assert.ok(base.injectedSecrets.has("acme"));
+  assert.ok(base.injectedSecrets.has("orb:acme"));
   await composed.destroyContainer(REQUEST);
-  assert.equal(base.containers.has("acme"), false);
+  assert.equal(base.containers.has("orb:acme"), false);
   await composed.revokeSecrets(REQUEST);
-  assert.equal(base.injectedSecrets.has("acme"), false);
+  assert.equal(base.injectedSecrets.has("orb:acme"), false);
 });
 
 test("withRealContainerDriver: overrides createContainer/destroyContainer/containerExists, forwards every other step to base", async () => {
@@ -110,13 +110,13 @@ test("withRealContainerDriver: overrides createContainer/destroyContainer/contai
   assert.deepEqual(calls, ["real-create", "real-exists", "real-destroy"]);
   // The fake's own createContainer never ran -- its `containers` set stays empty even though the composed
   // driver's own lifecycle calls all resolved successfully.
-  assert.equal(base.containers.has("acme"), false);
+  assert.equal(base.containers.has("orb:acme"), false);
 
   // Every non-container step still runs against `base` exactly as before composition.
   const details = await composed.provisionDatabase(REQUEST);
   assert.equal(details.host, "fake-acme.control-plane.invalid");
   await composed.injectSecrets(REQUEST);
-  assert.ok(base.injectedSecrets.has("acme"));
+  assert.ok(base.injectedSecrets.has("orb:acme"));
 });
 
 test("createTenantProvisioningDriver: falls back to the fake container behavior when containerBindings is omitted or empty", async () => {

control-plane/test/provisioning.test.ts

@@ -37,8 +37,8 @@ test("provisionTenant runs the three #7180 steps in order and reports the tenant
   );
   // Container "exists"/reachable via the fake after provision.
   assert.equal(await driver.containerExists({ tenant, product: "orb" }), true);
-  assert.ok(driver.databases.has("acme"));
-  assert.ok(driver.injectedSecrets.has("acme"));
+  assert.ok(driver.databases.has("orb:acme"));
+  assert.ok(driver.injectedSecrets.has("orb:acme"));
 });
 
 test("full lifecycle: provision → container exists → deprovision → container gone", async () => {
@@ -52,8 +52,8 @@ test("full lifecycle: provision → container exists → deprovision → contain
 
   assert.deepEqual(result, { tenant, product: "ams", state: "torn down" });
   assert.equal(await driver.containerExists({ tenant, product: "ams" }), false);
-  assert.equal(driver.databases.has("acme"), false);
-  assert.equal(driver.injectedSecrets.has("acme"), false);
+  assert.equal(driver.databases.has("ams:acme"), false);
+  assert.equal(driver.injectedSecrets.has("ams:acme"), false);
 });
 
 test("deprovisionTenant tears the steps down in reverse order", async () => {
@@ -83,7 +83,7 @@ test("deprovisionTenant on a never-provisioned tenant is a safe no-op that still
 
   assert.deepEqual(result, { tenant, product: "ams", state: "torn down" });
   assert.equal(await driver.containerExists({ tenant, product: "ams" }), false);
-  assert.equal(driver.containers.has("ghost"), false);
+  assert.equal(driver.containers.has("ams:ghost"), false);
 });
 
 test("the call shape is identical for an ORB tenant and an AMS tenant (product-agnostic)", async () => {