Allow overlapping Bot audiences in shared-app approvals

CHANGELOG.md

@@ -8,6 +8,12 @@ Newest first. `Unreleased` is what is on `main` and not yet tagged.
 
 ## Unreleased
 
+### A group reached in two ways can be approved for a shared app
+
+Approving a Bot for a shared app now succeeds when the same group appears in its published audience
+and its assigned groups. Repeated people or groups are stored once; a person and a group with the
+same name remain separate, and no additional audience is approved.
+
 **Before upgrading.** Sixteen things change for an existing deployment:
 - Bots now share one computer per person instead of each having its own. Migration
   `0069_isolated_bots` adds `agent_profiles.isolated`, off for every existing Bot. With the

server/src/plugins/shared-use-store.ts

@@ -284,13 +284,25 @@ export function createSharedUseStore(
         input.approval.audience === "people" &&
         input.approval.members.length > 0
       ) {
-        await tx.insert(sharedUseApprovalMembers).values(
-          input.approval.members.map((member) => ({
-            agentId: input.botId,
-            serverId,
-            ...member,
-          })),
-        );
+        // Publication and assignment can name the same group. Membership is a set of exact
+        // (kind, value) pairs; a user and a group with the same value are still distinct.
+        await tx
+          .insert(sharedUseApprovalMembers)
+          .values(
+            input.approval.members.map((member) => ({
+              agentId: input.botId,
+              serverId,
+              ...member,
+            })),
+          )
+          .onConflictDoNothing({
+            target: [
+              sharedUseApprovalMembers.agentId,
+              sharedUseApprovalMembers.serverId,
+              sharedUseApprovalMembers.kind,
+              sharedUseApprovalMembers.value,
+            ],
+          });
       }
       /* A pending request this approval now answers is closed as approved, so the inbox never asks twice. */
       const pending = await tx

server/tests/shared-use-store.integration.test.ts

@@ -12,6 +12,7 @@ import {
   responsibilities,
   sharedUseRequests,
   teamBotAudience,
+  teamBotAssignments,
   teamBotPublications,
   users,
 } from "../src/db/schema";
@@ -273,6 +274,64 @@ describe("the shared-use gate", () => {
   });
 });
 
+describe("approval members", () => {
+  test("repeated people and groups store one exact membership without widening access", async () => {
+    await store.setApproval({
+      botId: bot,
+      serverId: app,
+      by: id("owner"),
+      approval: {
+        audience: "people",
+        outsideInput: false,
+        members: [
+          { kind: "user", value: id("named") },
+          { kind: "user", value: id("named") },
+          { kind: "group", value: "platform" },
+          { kind: "group", value: "platform" },
+          { kind: "group", value: id("named") },
+        ],
+      },
+    });
+    const approval = await store.approvalFor(bot, app);
+    expect(approval?.members).toHaveLength(3);
+    expect(approval?.members).toEqual(
+      expect.arrayContaining([
+        { kind: "user", value: id("named") },
+        { kind: "group", value: "platform" },
+        { kind: "group", value: id("named") },
+      ]),
+    );
+    expect((await call("named")).allowed).toBe(true);
+    expect((await call("newhire")).allowed).toBe(true);
+    expect((await call("stranger")).allowed).toBe(false);
+  });
+
+  test("an audience also assigned to the Bot can be approved", async () => {
+    await database
+      .insert(teamBotPublications)
+      .values({ agentId: bot, publishedBy: id("owner"), audience: "people" });
+    await database
+      .insert(teamBotAudience)
+      .values({ agentId: bot, kind: "group", value: "platform" });
+    await database
+      .insert(teamBotAssignments)
+      .values({ agentId: bot, groupName: "platform", assignedBy: id("owner") });
+    await call("newhire");
+    const [pending] = await store.pendingFor(bot);
+    expect(pending?.proposed.members).toHaveLength(2);
+    await store.decide({
+      id: pending!.id,
+      by: id("admin"),
+      decision: "approve",
+    });
+    expect((await store.approvalFor(bot, app))?.members).toEqual([
+      { kind: "group", value: "platform" },
+    ]);
+    expect((await call("newhire")).allowed).toBe(true);
+    expect((await call("stranger")).allowed).toBe(false);
+  });
+});
+
 describe("decide", () => {
   test("two admins deciding one request record exactly one decision", async () => {
     await call("stranger");