From faf8bb65f49cfdf1810abb50915580c98f9fda6d Mon Sep 17 00:00:00 2001
From: Brian O'Kelley <bokelley@scope3.com>
Date: Wed, 22 Apr 2026 00:22:44 -0400
Subject: [PATCH 1/3] fix(visibility): close tier-gate blockers from #2793
 review
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

#2793 (three-tier agent visibility) shipped before the EmmaLouise2018
review was addressed. This PR closes the three Blocker / two High
findings.

- POST /api/me/member-profile (create) persisted agents verbatim, so
  an Explorer could land visibility:'public' on first write and later
  readers filtered strictly on === 'public' with no tier re-check.
  Extracted coercion from PUT into a shared agent-visibility-gate
  helper; both paths use it. Create response surfaces structured
  warnings[] on downgrade, matching PUT.

- MCP addAgent (member-tools.ts) defaulted to visibility:'public'
  with no tier gate. Changed default to 'members_only' — safer than
  resolving caller tier inline and matches the UI default for new
  agents.

- applyAgentVisibility had a TOCTOU: the outer requireApiAccessTier
  ran before the transaction. A concurrent Stripe downgrade could
  commit between the outer check and the UPDATE, letting a 'public'
  write land on a now-downgraded org. Now re-reads membership tier
  inside the profile FOR UPDATE transaction and rolls back with 403
  if the current tier lacks API access.

- demotePublicAgentsOnTierDowngrade called getProfileByOrgId then
  updateProfileByOrgId as two separate statements — no row lock.
  Rewritten to hold a pg client, BEGIN, SELECT FOR UPDATE on
  member_profiles, UPDATE, COMMIT. Combined with the inner-tx tier
  check above, a publish and a demote cannot interleave past each
  other's lock on the same profile.

Tests:
- New agent-visibility-gate.test.ts — 10 scenarios for the shared
  helper
- agent-visibility-enforcement.test.ts rewritten to mock getPool()
  and the pg client (9 scenarios, including "commits profile tx
  before touching brand.json")
- Full server unit suite green

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 .../fix-visibility-tier-gate-blockers.md      |  11 +
 server/src/addie/mcp/member-tools.ts          |   9 +-
 server/src/routes/member-profiles.ts          |  95 +++++----
 .../services/agent-visibility-enforcement.ts  |  89 +++++++-
 server/src/services/agent-visibility-gate.ts  |  59 ++++++
 .../unit/agent-visibility-enforcement.test.ts | 191 +++++++++++++-----
 .../tests/unit/agent-visibility-gate.test.ts  | 122 +++++++++++
 7 files changed, 481 insertions(+), 95 deletions(-)
 create mode 100644 .changeset/fix-visibility-tier-gate-blockers.md
 create mode 100644 server/src/services/agent-visibility-gate.ts
 create mode 100644 server/tests/unit/agent-visibility-gate.test.ts

diff --git a/.changeset/fix-visibility-tier-gate-blockers.md b/.changeset/fix-visibility-tier-gate-blockers.md
new file mode 100644
index 0000000000..33acaea934
--- /dev/null
+++ b/.changeset/fix-visibility-tier-gate-blockers.md
@@ -0,0 +1,11 @@
+---
+---
+
+Close the three-tier agent visibility blockers flagged in the PR #2793 review that landed before the review was addressed.
+
+- **POST /api/me/member-profile (create) now runs the same tier gate as PUT.** A fresh profile containing `visibility: 'public'` on an Explorer-tier org was accepted verbatim, then filtered-through strictly without a tier re-check. Extracted the coercion into a shared helper (`agent-visibility-gate`), wired into both paths, now returns structured `warnings[]` on the create response too.
+- **MCP `addAgent` now defaults to `members_only`** instead of `public`. Callers without an API-access tier could implicitly publish an agent via Addie, bypassing the explicit `/publish` tier check.
+- **`applyAgentVisibility` re-reads the membership tier inside its transaction.** The outer `requireApiAccessTier` check is a fast-fail only; a concurrent Stripe downgrade committing between the outer read and the profile UPDATE would otherwise slip a `public` write past the gate.
+- **`demotePublicAgentsOnTierDowngrade` now wraps the profile lock + update in a single transaction** with `SELECT ... FOR UPDATE`. Previously it read and wrote the profile as two statements, so a concurrent PATCH could reinsert a `public` entry between them. The two transactional guarantees together (apply + demote) mean a tier-gated publish and a tier downgrade can't interleave past each other's lock.
+
+New unit tests cover the shared gate helper and the transactional demote path; existing enforcement tests were rewritten to exercise the new pg client contract. Full unit suite green (1811 pass).
diff --git a/server/src/addie/mcp/member-tools.ts b/server/src/addie/mcp/member-tools.ts
index 14f5060473..9f3e596d7b 100644
--- a/server/src/addie/mcp/member-tools.ts
+++ b/server/src/addie/mcp/member-tools.ts
@@ -4903,7 +4903,14 @@ export function createMemberToolHandlers(
         if (profile) {
           const agents = profile.agents || [];
           if (!agents.some((a: any) => a.url === agentUrl)) {
-            agents.push({ url: agentUrl, name: displayName, visibility: 'public' });
+            // Default to members_only, not public. The public directory
+            // requires an API-access tier (Professional+); defaulting to
+            // 'public' here lets Addie implicitly publish an agent for an
+            // Explorer-tier caller who hasn't been tier-gated. Members_only
+            // keeps the agent discoverable to peer members with API access
+            // and lets the owner promote to public through the explicit,
+            // tier-checked /publish route when eligible.
+            agents.push({ url: agentUrl, name: displayName, visibility: 'members_only' });
             await memberDb.updateProfile(profile.id, { agents });
           }
         }
diff --git a/server/src/routes/member-profiles.ts b/server/src/routes/member-profiles.ts
index 062c067d67..3374dd36f8 100644
--- a/server/src/routes/member-profiles.ts
+++ b/server/src/routes/member-profiles.ts
@@ -27,6 +27,7 @@ import type { MemberBrandInfo, AgentVisibility, AgentConfig } from "../types.js"
 import type { CrawlerService } from "../crawler.js";
 import { validateCrawlDomain } from "../utils/url-security.js";
 import { recordProfilePublishedIfNeeded } from "../services/profile-publish-event.js";
+import { gateAgentVisibilityForCaller, type VisibilityWarning } from "../services/agent-visibility-gate.js";
 
 const orgKnowledgeDb = new OrgKnowledgeDatabase();
 
@@ -310,6 +311,16 @@ export function createMemberProfileRouter(config: MemberProfileRoutesConfig): Ro
         }
       }
 
+      // Gate agent visibility on create using the same helper the PUT
+      // path uses. Without this, an Explorer-tier user creating their
+      // first profile can land `visibility: 'public'` directly in the
+      // JSONB — subsequent readers filter strictly on `=== 'public'`
+      // with no tier re-check, so the entry stays public forever.
+      const createOrgForTier = await orgDb.getOrganization(targetOrgId);
+      const createCallerHasApi = hasApiAccess(resolveMembershipTier(createOrgForTier));
+      const { agents: gatedAgents, warnings: createWarnings } =
+        gateAgentVisibilityForCaller(agents, createCallerHasApi);
+
       const profile = await memberDb.createProfile({
         workos_organization_id: targetOrgId,
         display_name,
@@ -323,7 +334,7 @@ export function createMemberProfileRouter(config: MemberProfileRoutesConfig): Ro
         linkedin_url,
         twitter_url,
         offerings: offerings || [],
-        agents: agents || [],
+        agents: gatedAgents,
         headquarters,
         markets: markets || [],
         tags: tags || [],
@@ -385,7 +396,10 @@ export function createMemberProfileRouter(config: MemberProfileRoutesConfig): Ro
 
       logger.info({ profileId: profile.id, orgId: targetOrgId, slug, durationMs: Date.now() - startTime }, 'POST /api/me/member-profile completed');
 
-      res.status(201).json({ profile });
+      res.status(201).json({
+        profile,
+        ...(createWarnings.length ? { warnings: createWarnings } : {}),
+      });
     } catch (error) {
       logger.error({ err: error, durationMs: Date.now() - startTime }, 'POST /api/me/member-profile error');
       res.status(500).json({
@@ -496,41 +510,15 @@ export function createMemberProfileRouter(config: MemberProfileRoutesConfig): Ro
       // Enforce the tier gate on agent visibility so bulk-profile updates
       // cannot bypass the per-agent PATCH. Non-API-access callers may only
       // set 'private' or 'members_only' on any agent in the array; when
-      // they send 'public' we downgrade and tell them we did.
-      const warnings: Array<Record<string, unknown>> = [];
+      // they send 'public' we downgrade and tell them we did. Shared with
+      // the POST create path via gateAgentVisibilityForCaller.
+      let warnings: VisibilityWarning[] = [];
       if (Array.isArray(updates.agents)) {
         const localOrgForTier = await orgDb.getOrganization(targetOrgId);
         const callerHasApi = hasApiAccess(resolveMembershipTier(localOrgForTier));
-        updates.agents = updates.agents.map((raw: unknown) => {
-          const a = (raw ?? {}) as Record<string, unknown>;
-          const requested = a.visibility;
-          let visibility: AgentVisibility;
-          if (isValidAgentVisibility(requested)) {
-            visibility = requested;
-          } else if (a.is_public === true) {
-            visibility = 'public';
-          } else {
-            visibility = 'private';
-          }
-          if (visibility === 'public' && !callerHasApi) {
-            warnings.push({
-              code: 'visibility_downgraded',
-              agent_url: a.url,
-              requested: 'public',
-              applied: 'members_only',
-              reason: 'tier_required',
-              message: 'Publicly listing an agent requires Professional tier or higher; stored as members_only instead.',
-            });
-            visibility = 'members_only';
-          }
-          const cleaned: Record<string, unknown> = {
-            url: a.url,
-            visibility,
-          };
-          if (typeof a.name === 'string') cleaned.name = a.name;
-          if (typeof a.type === 'string') cleaned.type = a.type;
-          return cleaned;
-        });
+        const gated = gateAgentVisibilityForCaller(updates.agents, callerHasApi);
+        updates.agents = gated.agents;
+        warnings = gated.warnings;
       }
 
       const profile = await memberDb.updateProfileByOrgId(targetOrgId, updates);
@@ -649,6 +637,7 @@ export function createMemberProfileRouter(config: MemberProfileRoutesConfig): Ro
   ): Promise<
     | { status: 404; body: { error: string } }
     | { status: 400; body: { error: string } }
+    | { status: 403; body: { error: string; message: string } }
     | { status: 200; body: Record<string, unknown> }
   > {
     const pool = getPool();
@@ -667,6 +656,44 @@ export function createMemberProfileRouter(config: MemberProfileRoutesConfig): Ro
         await client.query('ROLLBACK');
         return { status: 404, body: { error: 'Profile not found' } };
       }
+
+      // Re-read tier INSIDE the transaction when the caller is trying
+      // to publish. The outer `requireApiAccessTier` check is a
+      // fast-fail for UX, but it reads the org row before this tx
+      // starts — a concurrent Stripe downgrade webhook can commit
+      // between the outer check and this UPDATE, letting a `public`
+      // write land on an org that's no longer API-access. Reading the
+      // tier-relevant columns here closes that window: if the org was
+      // downgraded after our outer check, we see the new state and
+      // ROLLBACK. The Stripe demote path locks member_profiles via
+      // FOR UPDATE, so it can't interleave past our own lock.
+      if (target === 'public') {
+        const orgRow = await client.query<{
+          membership_tier: string | null;
+          subscription_price_lookup_key: string | null;
+          subscription_status: string | null;
+          subscription_amount: number | null;
+          subscription_interval: string | null;
+          is_personal: boolean;
+        }>(
+          `SELECT membership_tier, subscription_price_lookup_key, subscription_status,
+                  subscription_amount, subscription_interval, is_personal
+           FROM organizations
+           WHERE workos_organization_id = $1`,
+          [orgId]
+        );
+        const currentTier = resolveMembershipTier(orgRow.rows[0] ?? null);
+        if (!hasApiAccess(currentTier)) {
+          await client.query('ROLLBACK');
+          return {
+            status: 403,
+            body: {
+              error: 'tier_required',
+              message: 'Publicly listing an agent requires Professional tier or higher.',
+            },
+          };
+        }
+      }
       const row = profileRow.rows[0] as { id: string; agents: unknown; primary_brand_domain: string | null };
       const parsedAgents = typeof row.agents === 'string'
         ? JSON.parse(row.agents)
diff --git a/server/src/services/agent-visibility-enforcement.ts b/server/src/services/agent-visibility-enforcement.ts
index 2ff9559a5b..76a3e2304e 100644
--- a/server/src/services/agent-visibility-enforcement.ts
+++ b/server/src/services/agent-visibility-enforcement.ts
@@ -8,6 +8,7 @@
 
 import { MemberDatabase } from '../db/member-db.js';
 import { BrandDatabase } from '../db/brand-db.js';
+import { getPool } from '../db/client.js';
 import {
   hasApiAccess,
   type MembershipTier,
@@ -29,6 +30,15 @@ export interface DemoteResult {
  * strip them from the org's primary brand.json manifest.
  *
  * Returns null when no action was taken (no downgrade, or no profile).
+ *
+ * The profile read + write runs inside a single transaction with
+ * `SELECT ... FOR UPDATE` on the profile row, so a concurrent PUT /
+ * PATCH / publish against the same profile can't reinsert a `public`
+ * entry between our read and write. The inner tier check in
+ * `applyAgentVisibility` (#2793 follow-up) is the other side of this
+ * invariant: it blocks on the same profile row before committing a
+ * new `public` write, so the two paths can't interleave past each
+ * other's lock.
  */
 export async function demotePublicAgentsOnTierDowngrade(
   orgId: string,
@@ -40,19 +50,73 @@ export async function demotePublicAgentsOnTierDowngrade(
   if (!hasApiAccess(oldTier)) return null;
   if (hasApiAccess(newTier)) return null;
 
-  const profile = await memberDb.getProfileByOrgId(orgId);
-  if (!profile) return null;
+  const pool = getPool();
+  const client = await pool.connect();
+  let profile: { id: string; agents: AgentConfig[]; primary_brand_domain: string | null } | null = null;
+  let demotedUrls = new Set<string>();
+  try {
+    await client.query('BEGIN');
 
-  const agents = profile.agents || [];
-  const publicAgents = agents.filter((a) => a.visibility === 'public');
-  if (publicAgents.length === 0) return null;
+    const row = await client.query<{
+      id: string;
+      agents: unknown;
+      primary_brand_domain: string | null;
+    }>(
+      `SELECT id, agents, primary_brand_domain
+       FROM member_profiles
+       WHERE workos_organization_id = $1
+       FOR UPDATE`,
+      [orgId],
+    );
+    if (row.rowCount === 0) {
+      await client.query('ROLLBACK');
+      return null;
+    }
+    const r = row.rows[0];
+    const parsedAgents = typeof r.agents === 'string'
+      ? JSON.parse(r.agents)
+      : Array.isArray(r.agents) ? r.agents : [];
+    const agents: AgentConfig[] = (parsedAgents as unknown[]).map((a) => {
+      const o = (a ?? {}) as Record<string, unknown>;
+      const v = o.visibility;
+      const visibility = v === 'private' || v === 'members_only' || v === 'public'
+        ? v
+        : o.is_public === true ? 'public' : 'private';
+      return {
+        url: String(o.url ?? ''),
+        visibility,
+        ...(typeof o.name === 'string' ? { name: o.name } : {}),
+        ...(typeof o.type === 'string' ? { type: o.type as AgentConfig['type'] } : {}),
+      };
+    });
 
-  const demotedUrls = new Set(publicAgents.map((a) => a.url));
-  const updatedAgents: AgentConfig[] = agents.map((a) =>
-    demotedUrls.has(a.url) ? { ...a, visibility: 'members_only' as const } : a
-  );
+    const publicAgents = agents.filter((a) => a.visibility === 'public');
+    if (publicAgents.length === 0) {
+      await client.query('ROLLBACK');
+      return null;
+    }
 
-  await memberDb.updateProfileByOrgId(orgId, { agents: updatedAgents });
+    demotedUrls = new Set(publicAgents.map((a) => a.url));
+    const updatedAgents: AgentConfig[] = agents.map((a) =>
+      demotedUrls.has(a.url) ? { ...a, visibility: 'members_only' as const } : a
+    );
+    profile = { id: r.id, agents: updatedAgents, primary_brand_domain: r.primary_brand_domain };
+
+    await client.query(
+      `UPDATE member_profiles
+       SET agents = $1::jsonb, updated_at = NOW()
+       WHERE id = $2`,
+      [JSON.stringify(updatedAgents), r.id],
+    );
+    await client.query('COMMIT');
+  } catch (err) {
+    await client.query('ROLLBACK').catch(() => {});
+    throw err;
+  } finally {
+    client.release();
+  }
+
+  if (!profile) return null;
 
   let brandJsonCleared = false;
   if (profile.primary_brand_domain) {
@@ -80,5 +144,10 @@ export async function demotePublicAgentsOnTierDowngrade(
     'Demoted public agents to members_only after tier downgrade'
   );
 
+  // Kept for compatibility — MemberDatabase / BrandDatabase args are no
+  // longer used for the transactional path, but callers that pass them
+  // for testing shouldn't need updating yet.
+  void memberDb;
+
   return { orgId, demotedCount: demotedUrls.size, brandJsonCleared };
 }
diff --git a/server/src/services/agent-visibility-gate.ts b/server/src/services/agent-visibility-gate.ts
new file mode 100644
index 0000000000..b3f845d997
--- /dev/null
+++ b/server/src/services/agent-visibility-gate.ts
@@ -0,0 +1,59 @@
+/**
+ * Normalize a caller-supplied agents[] and enforce the visibility tier
+ * gate. Non-API-access callers cannot set `visibility: 'public'`; any
+ * such entries are downgraded to `members_only` and a structured warning
+ * is emitted so the caller knows we coerced them. Shared between the
+ * POST (create) and PUT (update) paths of /api/me/member-profile so
+ * neither can be smuggled past the per-agent /publish tier check.
+ */
+
+import { isValidAgentVisibility } from '../types.js';
+import type { AgentConfig, AgentVisibility } from '../types.js';
+
+export interface VisibilityWarning {
+  code: 'visibility_downgraded';
+  agent_url: unknown;
+  requested: 'public';
+  applied: 'members_only';
+  reason: 'tier_required';
+  message: string;
+}
+
+export function gateAgentVisibilityForCaller(
+  rawAgents: unknown,
+  callerHasApi: boolean,
+): { agents: AgentConfig[]; warnings: VisibilityWarning[] } {
+  if (!Array.isArray(rawAgents)) return { agents: [], warnings: [] };
+  const warnings: VisibilityWarning[] = [];
+  const agents = rawAgents.map((raw: unknown) => {
+    const a = (raw ?? {}) as Record<string, unknown>;
+    const requested = a.visibility;
+    let visibility: AgentVisibility;
+    if (isValidAgentVisibility(requested)) {
+      visibility = requested;
+    } else if (a.is_public === true) {
+      visibility = 'public';
+    } else {
+      visibility = 'private';
+    }
+    if (visibility === 'public' && !callerHasApi) {
+      warnings.push({
+        code: 'visibility_downgraded',
+        agent_url: a.url,
+        requested: 'public',
+        applied: 'members_only',
+        reason: 'tier_required',
+        message: 'Publicly listing an agent requires Professional tier or higher; stored as members_only instead.',
+      });
+      visibility = 'members_only';
+    }
+    const cleaned: AgentConfig = {
+      url: String(a.url ?? ''),
+      visibility,
+      ...(typeof a.name === 'string' ? { name: a.name } : {}),
+      ...(typeof a.type === 'string' ? { type: a.type as AgentConfig['type'] } : {}),
+    };
+    return cleaned;
+  });
+  return { agents, warnings };
+}
diff --git a/server/tests/unit/agent-visibility-enforcement.test.ts b/server/tests/unit/agent-visibility-enforcement.test.ts
index 3201aeb1c8..eb0a2e650e 100644
--- a/server/tests/unit/agent-visibility-enforcement.test.ts
+++ b/server/tests/unit/agent-visibility-enforcement.test.ts
@@ -1,23 +1,75 @@
 import { describe, it, expect, vi, beforeEach } from 'vitest';
 import type { AgentConfig } from '../../src/types.js';
 
+// `demotePublicAgentsOnTierDowngrade` now uses `getPool()` + raw SQL with
+// `SELECT … FOR UPDATE` so the read and write can't interleave with a
+// concurrent publish. The mocks below emulate the client contract that
+// the service depends on: BEGIN / SELECT / UPDATE / COMMIT / ROLLBACK.
+vi.mock('../../src/db/client.js', () => {
+  const release = vi.fn();
+  const client: { query: ReturnType<typeof vi.fn>; release: typeof release } = {
+    query: vi.fn(),
+    release,
+  };
+  const connect = vi.fn(async () => client);
+  return {
+    getPool: () => ({ connect }),
+    query: vi.fn(),
+    // test-accessor so each test can reconfigure the client's query stub
+    __client: client,
+    __connect: connect,
+  };
+});
+
+import { demotePublicAgentsOnTierDowngrade } from '../../src/services/agent-visibility-enforcement.js';
+// @ts-expect-error — internal test accessors exposed by the vi.mock above
+import { __client, __connect } from '../../src/db/client.js';
+
+type SelectRow = {
+  id: string;
+  agents: unknown;
+  primary_brand_domain: string | null;
+};
+
+/**
+ * Wire the pg client mock so the tx flow (BEGIN → SELECT FOR UPDATE →
+ * UPDATE → COMMIT) behaves as described, and capture the UPDATE args
+ * for assertion.
+ */
+function mockProfileTx(rows: SelectRow[]): { updateArgs: unknown[][] } {
+  const updateArgs: unknown[][] = [];
+  __client.query.mockReset();
+  __client.release.mockReset();
+  __client.query.mockImplementation(async (sql: string, params?: unknown[]) => {
+    if (sql === 'BEGIN' || sql === 'COMMIT' || sql === 'ROLLBACK') {
+      return { rowCount: 0, rows: [] };
+    }
+    if (sql.includes('SELECT id, agents, primary_brand_domain')) {
+      return { rowCount: rows.length, rows };
+    }
+    if (sql.trim().startsWith('UPDATE member_profiles')) {
+      updateArgs.push(params ?? []);
+      return { rowCount: 1, rows: [] };
+    }
+    throw new Error(`Unexpected query: ${sql}`);
+  });
+  return { updateArgs };
+}
+
 describe('demotePublicAgentsOnTierDowngrade', () => {
   let memberDb: any;
   let brandDb: any;
-  let demote: typeof import('../../src/services/agent-visibility-enforcement.js').demotePublicAgentsOnTierDowngrade;
 
-  beforeEach(async () => {
+  beforeEach(() => {
     memberDb = {
       getProfileByOrgId: vi.fn(),
-      updateProfileByOrgId: vi.fn().mockResolvedValue(null),
+      updateProfileByOrgId: vi.fn(),
     };
     brandDb = {
       getDiscoveredBrandByDomain: vi.fn(),
       updateManifestAgents: vi.fn().mockResolvedValue(undefined),
     };
-    ({ demotePublicAgentsOnTierDowngrade: demote } = await import(
-      '../../src/services/agent-visibility-enforcement.js'
-    ));
+    __connect.mockClear();
   });
 
   function agent(url: string, visibility: AgentConfig['visibility']): AgentConfig {
@@ -25,68 +77,83 @@ describe('demotePublicAgentsOnTierDowngrade', () => {
   }
 
   it('no-op when old tier had no API access', async () => {
-    const result = await demote('org1', 'individual_academic', null, memberDb, brandDb);
+    const result = await demotePublicAgentsOnTierDowngrade(
+      'org1', 'individual_academic', null, memberDb, brandDb,
+    );
     expect(result).toBeNull();
-    expect(memberDb.getProfileByOrgId).not.toHaveBeenCalled();
+    expect(__connect).not.toHaveBeenCalled();
   });
 
   it('no-op when new tier still has API access', async () => {
-    const result = await demote('org1', 'company_icl', 'individual_professional', memberDb, brandDb);
+    const result = await demotePublicAgentsOnTierDowngrade(
+      'org1', 'company_icl', 'individual_professional', memberDb, brandDb,
+    );
     expect(result).toBeNull();
-    expect(memberDb.getProfileByOrgId).not.toHaveBeenCalled();
+    expect(__connect).not.toHaveBeenCalled();
   });
 
   it('no-op when org has no profile', async () => {
-    memberDb.getProfileByOrgId.mockResolvedValue(null);
-    const result = await demote('org1', 'individual_professional', null, memberDb, brandDb);
+    mockProfileTx([]);
+    const result = await demotePublicAgentsOnTierDowngrade(
+      'org1', 'individual_professional', null, memberDb, brandDb,
+    );
     expect(result).toBeNull();
-    expect(memberDb.updateProfileByOrgId).not.toHaveBeenCalled();
+    // Should open a tx, see 0 rows, ROLLBACK, not UPDATE
+    const queries = __client.query.mock.calls.map((c: any[]) => c[0]);
+    expect(queries).toContain('BEGIN');
+    expect(queries).toContain('ROLLBACK');
+    expect(queries.some((q: string) => q.startsWith?.('UPDATE'))).toBe(false);
   });
 
   it('no-op when profile has no public agents', async () => {
-    memberDb.getProfileByOrgId.mockResolvedValue({
-      agents: [agent('https://a.example', 'private'), agent('https://b.example', 'members_only')],
-      primary_brand_domain: null,
-    });
-    const result = await demote('org1', 'individual_professional', 'individual_academic', memberDb, brandDb);
+    const agents = [agent('https://a.example', 'private'), agent('https://b.example', 'members_only')];
+    mockProfileTx([{ id: 'p1', agents, primary_brand_domain: null }]);
+    const result = await demotePublicAgentsOnTierDowngrade(
+      'org1', 'individual_professional', 'individual_academic', memberDb, brandDb,
+    );
     expect(result).toBeNull();
-    expect(memberDb.updateProfileByOrgId).not.toHaveBeenCalled();
+    const queries = __client.query.mock.calls.map((c: any[]) => c[0]);
+    expect(queries).toContain('ROLLBACK');
+    expect(queries.some((q: string) => q.trim?.().startsWith('UPDATE member_profiles'))).toBe(false);
   });
 
   it('demotes public agents to members_only on Professional → Explorer', async () => {
-    memberDb.getProfileByOrgId.mockResolvedValue({
-      agents: [
-        agent('https://pub.example', 'public'),
-        agent('https://mem.example', 'members_only'),
-        agent('https://priv.example', 'private'),
-      ],
-      primary_brand_domain: null,
-    });
-    const result = await demote('org1', 'individual_professional', 'individual_academic', memberDb, brandDb);
+    const agents = [
+      agent('https://pub.example', 'public'),
+      agent('https://mem.example', 'members_only'),
+      agent('https://priv.example', 'private'),
+    ];
+    const { updateArgs } = mockProfileTx([
+      { id: 'p1', agents, primary_brand_domain: null },
+    ]);
+    const result = await demotePublicAgentsOnTierDowngrade(
+      'org1', 'individual_professional', 'individual_academic', memberDb, brandDb,
+    );
     expect(result).toEqual({ orgId: 'org1', demotedCount: 1, brandJsonCleared: false });
-    expect(memberDb.updateProfileByOrgId).toHaveBeenCalledWith('org1', {
-      agents: [
-        agent('https://pub.example', 'members_only'),
-        agent('https://mem.example', 'members_only'),
-        agent('https://priv.example', 'private'),
-      ],
-    });
+    expect(updateArgs).toHaveLength(1);
+    // First UPDATE param is the new agents JSON; assert the public entry flipped.
+    const writtenAgents = JSON.parse(updateArgs[0][0] as string);
+    expect(writtenAgents).toEqual([
+      agent('https://pub.example', 'members_only'),
+      agent('https://mem.example', 'members_only'),
+      agent('https://priv.example', 'private'),
+    ]);
   });
 
   it('demotes on full cancellation (newTier = null)', async () => {
-    memberDb.getProfileByOrgId.mockResolvedValue({
-      agents: [agent('https://p.example', 'public')],
-      primary_brand_domain: null,
-    });
-    const result = await demote('org1', 'company_leader', null, memberDb, brandDb);
+    mockProfileTx([
+      { id: 'p1', agents: [agent('https://p.example', 'public')], primary_brand_domain: null },
+    ]);
+    const result = await demotePublicAgentsOnTierDowngrade(
+      'org1', 'company_leader', null, memberDb, brandDb,
+    );
     expect(result?.demotedCount).toBe(1);
   });
 
   it('clears demoted agents from a community brand.json', async () => {
-    memberDb.getProfileByOrgId.mockResolvedValue({
-      agents: [agent('https://p.example', 'public')],
-      primary_brand_domain: 'acme.example',
-    });
+    mockProfileTx([
+      { id: 'p1', agents: [agent('https://p.example', 'public')], primary_brand_domain: 'acme.example' },
+    ]);
     brandDb.getDiscoveredBrandByDomain.mockResolvedValue({
       source_type: 'community',
       brand_manifest: {
@@ -96,7 +163,9 @@ describe('demotePublicAgentsOnTierDowngrade', () => {
         ],
       },
     });
-    const result = await demote('org1', 'individual_professional', null, memberDb, brandDb);
+    const result = await demotePublicAgentsOnTierDowngrade(
+      'org1', 'individual_professional', null, memberDb, brandDb,
+    );
     expect(result?.brandJsonCleared).toBe(true);
     expect(brandDb.updateManifestAgents).toHaveBeenCalledWith(
       'acme.example',
@@ -106,18 +175,40 @@ describe('demotePublicAgentsOnTierDowngrade', () => {
   });
 
   it('does not touch brand.json for self-hosted brands', async () => {
-    memberDb.getProfileByOrgId.mockResolvedValue({
-      agents: [agent('https://p.example', 'public')],
-      primary_brand_domain: 'acme.example',
-    });
+    mockProfileTx([
+      { id: 'p1', agents: [agent('https://p.example', 'public')], primary_brand_domain: 'acme.example' },
+    ]);
     brandDb.getDiscoveredBrandByDomain.mockResolvedValue({
       source_type: 'brand_json',
       brand_manifest: {
         agents: [{ url: 'https://p.example', type: 'brand', id: 'p' }],
       },
     });
-    const result = await demote('org1', 'individual_professional', null, memberDb, brandDb);
+    const result = await demotePublicAgentsOnTierDowngrade(
+      'org1', 'individual_professional', null, memberDb, brandDb,
+    );
     expect(result?.brandJsonCleared).toBe(false);
     expect(brandDb.updateManifestAgents).not.toHaveBeenCalled();
   });
+
+  it('commits the profile tx before touching brand.json (so a failed manifest write does not orphan the JSONB update)', async () => {
+    mockProfileTx([
+      { id: 'p1', agents: [agent('https://p.example', 'public')], primary_brand_domain: 'acme.example' },
+    ]);
+    brandDb.getDiscoveredBrandByDomain.mockResolvedValue({
+      source_type: 'community',
+      brand_manifest: { agents: [{ url: 'https://p.example', type: 'brand', id: 'p' }] },
+    });
+    await demotePublicAgentsOnTierDowngrade(
+      'org1', 'individual_professional', null, memberDb, brandDb,
+    );
+    const queries = __client.query.mock.calls.map((c: any[]) => c[0]);
+    const commitAt = queries.indexOf('COMMIT');
+    expect(commitAt).toBeGreaterThan(-1);
+    // brand.json write happens after client.release() via the pool, so by
+    // design it can't be in __client.query.mock.calls — no need to assert
+    // ordering there. Instead, verify UPDATE ran before COMMIT.
+    const updateAt = queries.findIndex((q: string) => q.trim?.().startsWith('UPDATE member_profiles'));
+    expect(updateAt).toBeLessThan(commitAt);
+  });
 });
diff --git a/server/tests/unit/agent-visibility-gate.test.ts b/server/tests/unit/agent-visibility-gate.test.ts
new file mode 100644
index 0000000000..2d050a0210
--- /dev/null
+++ b/server/tests/unit/agent-visibility-gate.test.ts
@@ -0,0 +1,122 @@
+import { describe, it, expect } from 'vitest';
+import { gateAgentVisibilityForCaller } from '../../src/services/agent-visibility-gate.js';
+
+/**
+ * Shared-helper tests for the tier-gate coercion used by both POST and
+ * PUT of /api/me/member-profile. The reviewer flagged (PR #2793) that
+ * the PUT path did the gate but the POST path didn't — so an Explorer
+ * user creating their first profile could land `visibility: 'public'`
+ * on first write, and subsequent readers filtered strictly on
+ * `=== 'public'` without a tier re-check. This helper centralizes the
+ * coercion so neither path can drift.
+ */
+
+describe('gateAgentVisibilityForCaller', () => {
+  it('returns empty results for a non-array input', () => {
+    expect(gateAgentVisibilityForCaller(undefined, true)).toEqual({ agents: [], warnings: [] });
+    expect(gateAgentVisibilityForCaller(null, false)).toEqual({ agents: [], warnings: [] });
+    expect(gateAgentVisibilityForCaller('not an array', false)).toEqual({ agents: [], warnings: [] });
+  });
+
+  it('passes valid visibility values through when caller has API access', () => {
+    const { agents, warnings } = gateAgentVisibilityForCaller(
+      [
+        { url: 'https://a', visibility: 'public' },
+        { url: 'https://b', visibility: 'members_only' },
+        { url: 'https://c', visibility: 'private' },
+      ],
+      true,
+    );
+    expect(warnings).toEqual([]);
+    expect(agents).toEqual([
+      { url: 'https://a', visibility: 'public' },
+      { url: 'https://b', visibility: 'members_only' },
+      { url: 'https://c', visibility: 'private' },
+    ]);
+  });
+
+  it('downgrades public → members_only and emits a warning when caller lacks API access', () => {
+    const { agents, warnings } = gateAgentVisibilityForCaller(
+      [{ url: 'https://a', visibility: 'public' }],
+      false,
+    );
+    expect(agents).toEqual([{ url: 'https://a', visibility: 'members_only' }]);
+    expect(warnings).toHaveLength(1);
+    expect(warnings[0]).toMatchObject({
+      code: 'visibility_downgraded',
+      agent_url: 'https://a',
+      requested: 'public',
+      applied: 'members_only',
+      reason: 'tier_required',
+    });
+  });
+
+  it('accepts members_only from a non-API-access caller', () => {
+    const { agents, warnings } = gateAgentVisibilityForCaller(
+      [{ url: 'https://a', visibility: 'members_only' }],
+      false,
+    );
+    expect(agents).toEqual([{ url: 'https://a', visibility: 'members_only' }]);
+    expect(warnings).toEqual([]);
+  });
+
+  it('translates legacy is_public:true to public when visibility is missing', () => {
+    const { agents } = gateAgentVisibilityForCaller(
+      [{ url: 'https://a', is_public: true }],
+      true,
+    );
+    expect(agents[0].visibility).toBe('public');
+  });
+
+  it('translates legacy is_public:false (or missing) to private', () => {
+    const { agents } = gateAgentVisibilityForCaller(
+      [{ url: 'https://a' }, { url: 'https://b', is_public: false }],
+      true,
+    );
+    expect(agents[0].visibility).toBe('private');
+    expect(agents[1].visibility).toBe('private');
+  });
+
+  it('downgrades legacy is_public:true → members_only for a non-API-access caller', () => {
+    // If a legacy client sends is_public:true against a downgraded org,
+    // we still have to enforce the tier gate — not just for the new
+    // `visibility:'public'` attack surface.
+    const { agents, warnings } = gateAgentVisibilityForCaller(
+      [{ url: 'https://a', is_public: true }],
+      false,
+    );
+    expect(agents[0].visibility).toBe('members_only');
+    expect(warnings).toHaveLength(1);
+  });
+
+  it('rejects unknown visibility strings by falling back to legacy is_public (then private)', () => {
+    const { agents } = gateAgentVisibilityForCaller(
+      [{ url: 'https://a', visibility: 'admin' }],
+      true,
+    );
+    expect(agents[0].visibility).toBe('private');
+  });
+
+  it('strips unexpected fields from agent entries', () => {
+    const { agents } = gateAgentVisibilityForCaller(
+      [{ url: 'https://a', visibility: 'private', name: 'Agent', type: 'brand', password: 'hunter2' }],
+      true,
+    );
+    expect(agents[0]).toEqual({ url: 'https://a', visibility: 'private', name: 'Agent', type: 'brand' });
+    expect((agents[0] as any).password).toBeUndefined();
+  });
+
+  it('handles a mix of public and non-public agents in one call', () => {
+    const { agents, warnings } = gateAgentVisibilityForCaller(
+      [
+        { url: 'https://a', visibility: 'public' },
+        { url: 'https://b', visibility: 'private' },
+        { url: 'https://c', visibility: 'public' },
+      ],
+      false,
+    );
+    expect(agents.map((a) => a.visibility)).toEqual(['members_only', 'private', 'members_only']);
+    expect(warnings).toHaveLength(2);
+    expect(warnings.map((w) => w.agent_url)).toEqual(['https://a', 'https://c']);
+  });
+});

From 58a6420cf20c430eb5a96aa970c37e76d68b8f6a Mon Sep 17 00:00:00 2001
From: Brian O'Kelley <bokelley@scope3.com>
Date: Wed, 22 Apr 2026 00:34:47 -0400
Subject: [PATCH 2/3] fix(visibility): expert-review follow-ups on #2822
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Addresses Must-Fix and Should-Fix items from the three-expert review:

- DX Must-Fix: dashboard client silently dropped warnings[]. member-
  profile.html now renders a "saved, but N agents could not be listed
  publicly" banner with an upgrade CTA when the response carries
  downgrade warnings. Applies to both POST (new create flow) and PUT
  (existing update flow) — both paths return the same shape.
- DX Must-Fix: save_agent tool description didn't mention visibility.
  Added explicit members_only default + publish-flow semantics to the
  tool description, and Addie's save confirmation now names the
  visibility state instead of the generic "added to dashboard".
- Security Should-Fix: POST /api/me/member-profile and PUT bulk update
  accepted `is_public: true` verbatim — same smuggle class as the
  agent-visibility bug this PR fixes. Both paths now route the flag
  through orgDb.hasActiveSubscription, fall back to false, and emit a
  structured warning.
- Security Should-Fix: agents[].type was cast from arbitrary caller
  string into brand.json's manifest. Now gated through isValidAgentType
  and dropped if unknown.
- Code-reviewer Should-Fix: dead memberDb DI arg on
  demotePublicAgentsOnTierDowngrade removed. Enforcement test rewritten
  to assert the SELECT contains FOR UPDATE and the params[0] matches
  orgId (regression guard for the transactional invariant).
- DX Should-Fix: VisibilityWarning.agent_url typed as string and coerced
  at emit time — clients no longer risk rendering whatever raw shape
  arrived in the request body.

Out of scope, filed as follow-ups:
- #2825: brand.json write inside applyAgentVisibility's profile tx
- #2826: inner-tx tier column list duplicates resolveMembershipTier

Full server unit suite: 1814 pass.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 server/public/member-profile.html             | 28 +++++-
 server/src/addie/mcp/member-tools.ts          |  4 +-
 server/src/routes/member-profiles.ts          | 40 ++++++++-
 .../services/agent-visibility-enforcement.ts  |  7 --
 server/src/services/agent-visibility-gate.ts  | 21 +++--
 .../unit/agent-visibility-enforcement.test.ts | 90 ++++++++++---------
 6 files changed, 132 insertions(+), 58 deletions(-)

diff --git a/server/public/member-profile.html b/server/public/member-profile.html
index 199f5eb6af..9ad3b45fef 100644
--- a/server/public/member-profile.html
+++ b/server/public/member-profile.html
@@ -2242,15 +2242,37 @@ <h2>Tags</h2>
         const wasNewProfile = isNewProfile;
         isNewProfile = false;
 
+        // Server may have coerced visibility settings the caller isn't
+        // eligible for (e.g. public → members_only on a non-API-access
+        // tier). Surface those warnings so the user knows we didn't
+        // silently persist what they asked for.
+        const warnings = Array.isArray(data.warnings) ? data.warnings : [];
+        const downgrades = warnings.filter(w => w && w.code === 'visibility_downgraded');
+
         // If this was a new profile creation, redirect to dashboard with success message
         if (wasNewProfile) {
           const orgParam = currentOrgId ? `&org=${currentOrgId}` : '';
-          window.location.href = `/dashboard?profile_created=true${orgParam}`;
+          const warnParam = downgrades.length ? `&visibility_downgraded=${downgrades.length}` : '';
+          window.location.href = `/dashboard?profile_created=true${orgParam}${warnParam}`;
           return;
         }
 
-        // For updates, show success message and stay on page
-        showSuccess('Profile saved successfully!');
+        // For updates, show success message and stay on page.
+        // If visibility entries got downgraded, pin the reason above the
+        // generic banner so the user sees the upgrade CTA.
+        if (downgrades.length > 0) {
+          const agents = downgrades.map(w => w.agent_url).filter(Boolean);
+          const detail = agents.length
+            ? ` Affected agents: ${agents.join(', ')}.`
+            : '';
+          showError(
+            `Saved, but ${downgrades.length} agent${downgrades.length > 1 ? 's' : ''} could not be listed publicly — ` +
+            `public listing requires Professional tier or higher, so they were stored as members_only instead.${detail} ` +
+            `Upgrade at /membership to publish publicly.`
+          );
+        } else {
+          showSuccess('Profile saved successfully!');
+        }
         saveBtn.disabled = false;
         saveBtn.textContent = 'Save Profile';
 
diff --git a/server/src/addie/mcp/member-tools.ts b/server/src/addie/mcp/member-tools.ts
index 9f3e596d7b..8dd7c20b94 100644
--- a/server/src/addie/mcp/member-tools.ts
+++ b/server/src/addie/mcp/member-tools.ts
@@ -1088,7 +1088,7 @@ export const MEMBER_TOOLS: AddieTool[] = [
   {
     name: 'save_agent',
     description:
-      'Save an agent URL to the organization\'s context and add it to the dashboard for compliance monitoring. Optionally store credentials securely (encrypted, never shown in conversations). Three auth modes, any of which may be combined with a new or existing save: (1) static bearer/basic via `auth_token`, (2) OAuth 2.0 client credentials (RFC 6749 §4.4, machine-to-machine) via `oauth_client_credentials`. Use this when users want to connect their agent, set up compliance monitoring, save their agent for testing, or provide credentials.',
+      'Save an agent URL to the organization\'s context and add it to the dashboard for compliance monitoring. New agents land in the dashboard with `members_only` visibility — discoverable to fellow Professional-tier (or higher) members, but not publicly listed in the directory or brand.json. To list publicly, the caller promotes the agent via the dashboard publish flow; that flow gates on an API-access subscription tier. Optionally store credentials securely (encrypted, never shown in conversations). Three auth modes, any of which may be combined with a new or existing save: (1) static bearer/basic via `auth_token`, (2) OAuth 2.0 client credentials (RFC 6749 §4.4, machine-to-machine) via `oauth_client_credentials`. Use this when users want to connect their agent, set up compliance monitoring, save their agent for testing, or provide credentials.',
     usage_hints: 'use for "connect my agent", "add agent for compliance monitoring", "save my agent", "remember this agent URL", "store my auth token", "configure client credentials", "save OAuth client credentials"',
     input_schema: {
       type: 'object',
@@ -4984,7 +4984,7 @@ export function createMemberToolHandlers(
         response += `\n🔐 OAuth client-credentials saved securely for token endpoint ${clientCredentials.token_endpoint}\n`;
         response += `_The client secret is encrypted and will never be shown again. The SDK exchanges and refreshes at test time._\n`;
       }
-      response += `\nThe agent has been added to your dashboard. When you test this agent, I'll automatically use the saved credentials.`;
+      response += `\nThe agent has been added to your dashboard with **members_only** visibility — other Professional-tier members can discover it, but it won't appear in the public directory. To publish publicly, use the dashboard publish flow (requires a Professional or higher subscription). When you test this agent, I'll automatically use the saved credentials.`;
 
       return response;
     } catch (error) {
diff --git a/server/src/routes/member-profiles.ts b/server/src/routes/member-profiles.ts
index 3374dd36f8..9b3a4f302b 100644
--- a/server/src/routes/member-profiles.ts
+++ b/server/src/routes/member-profiles.ts
@@ -321,6 +321,26 @@ export function createMemberProfileRouter(config: MemberProfileRoutesConfig): Ro
       const { agents: gatedAgents, warnings: createWarnings } =
         gateAgentVisibilityForCaller(agents, createCallerHasApi);
 
+      // Same tier gate for the profile-level `is_public` flag. The
+      // `/visibility` PUT route gates this through hasActiveSubscription
+      // (line 1343), but the POST create and PUT bulk-update paths
+      // previously accepted the raw body value — same smuggle class as
+      // the agent-visibility bug this PR fixes.
+      let effectiveIsPublic = is_public === true;
+      if (effectiveIsPublic && !isDevModeEnabled()) {
+        if (!(await orgDb.hasActiveSubscription(targetOrgId))) {
+          effectiveIsPublic = false;
+          createWarnings.push({
+            code: 'visibility_downgraded',
+            agent_url: 'profile',
+            requested: 'public',
+            applied: 'members_only',
+            reason: 'tier_required',
+            message: 'Making the profile publicly visible requires an active paid membership; stored as private instead.',
+          });
+        }
+      }
+
       const profile = await memberDb.createProfile({
         workos_organization_id: targetOrgId,
         display_name,
@@ -338,7 +358,7 @@ export function createMemberProfileRouter(config: MemberProfileRoutesConfig): Ro
         headquarters,
         markets: markets || [],
         tags: tags || [],
-        is_public: is_public ?? false,
+        is_public: effectiveIsPublic,
         show_in_carousel: show_in_carousel ?? false,
       });
 
@@ -521,6 +541,24 @@ export function createMemberProfileRouter(config: MemberProfileRoutesConfig): Ro
         warnings = gated.warnings;
       }
 
+      // Same gate for the profile-level `is_public` flag. The dedicated
+      // `/visibility` PUT route already gates this; the bulk-profile
+      // update path previously forwarded the raw body value, reopening
+      // the same smuggle a non-paying caller could use on POST create.
+      if (updates.is_public === true && !isDevModeEnabled()) {
+        if (!(await orgDb.hasActiveSubscription(targetOrgId))) {
+          updates.is_public = false;
+          warnings.push({
+            code: 'visibility_downgraded',
+            agent_url: 'profile',
+            requested: 'public',
+            applied: 'members_only',
+            reason: 'tier_required',
+            message: 'Making the profile publicly visible requires an active paid membership; left as private.',
+          });
+        }
+      }
+
       const profile = await memberDb.updateProfileByOrgId(targetOrgId, updates);
 
       // Trigger crawl for new/updated publisher domains (fire-and-forget)
diff --git a/server/src/services/agent-visibility-enforcement.ts b/server/src/services/agent-visibility-enforcement.ts
index 76a3e2304e..71fe2d9591 100644
--- a/server/src/services/agent-visibility-enforcement.ts
+++ b/server/src/services/agent-visibility-enforcement.ts
@@ -6,7 +6,6 @@
  * API-access members while respecting the gate on public listing.
  */
 
-import { MemberDatabase } from '../db/member-db.js';
 import { BrandDatabase } from '../db/brand-db.js';
 import { getPool } from '../db/client.js';
 import {
@@ -44,7 +43,6 @@ export async function demotePublicAgentsOnTierDowngrade(
   orgId: string,
   oldTier: MembershipTier | null,
   newTier: MembershipTier | null,
-  memberDb: MemberDatabase = new MemberDatabase(),
   brandDb: BrandDatabase = new BrandDatabase(),
 ): Promise<DemoteResult | null> {
   if (!hasApiAccess(oldTier)) return null;
@@ -144,10 +142,5 @@ export async function demotePublicAgentsOnTierDowngrade(
     'Demoted public agents to members_only after tier downgrade'
   );
 
-  // Kept for compatibility — MemberDatabase / BrandDatabase args are no
-  // longer used for the transactional path, but callers that pass them
-  // for testing shouldn't need updating yet.
-  void memberDb;
-
   return { orgId, demotedCount: demotedUrls.size, brandJsonCleared };
 }
diff --git a/server/src/services/agent-visibility-gate.ts b/server/src/services/agent-visibility-gate.ts
index b3f845d997..6a365684cb 100644
--- a/server/src/services/agent-visibility-gate.ts
+++ b/server/src/services/agent-visibility-gate.ts
@@ -7,12 +7,18 @@
  * neither can be smuggled past the per-agent /publish tier check.
  */
 
-import { isValidAgentVisibility } from '../types.js';
+import { isValidAgentVisibility, isValidAgentType } from '../types.js';
 import type { AgentConfig, AgentVisibility } from '../types.js';
 
 export interface VisibilityWarning {
   code: 'visibility_downgraded';
-  agent_url: unknown;
+  /**
+   * String identifier for what got downgraded — an agent URL, or the
+   * sentinel `'profile'` when the profile-level `is_public` flag was
+   * the target. Coerced to string at emit time so the wire shape is
+   * trustworthy for clients that render it.
+   */
+  agent_url: string;
   requested: 'public';
   applied: 'members_only';
   reason: 'tier_required';
@@ -36,10 +42,11 @@ export function gateAgentVisibilityForCaller(
     } else {
       visibility = 'private';
     }
+    const url = typeof a.url === 'string' ? a.url : String(a.url ?? '');
     if (visibility === 'public' && !callerHasApi) {
       warnings.push({
         code: 'visibility_downgraded',
-        agent_url: a.url,
+        agent_url: url,
         requested: 'public',
         applied: 'members_only',
         reason: 'tier_required',
@@ -47,11 +54,15 @@ export function gateAgentVisibilityForCaller(
       });
       visibility = 'members_only';
     }
+    // Drop unknown `type` values instead of casting — the field flows
+    // into brand.json (`agentEntry.type`) so an arbitrary tenant string
+    // would become a durable artifact in other members' manifests.
+    const typeValue = typeof a.type === 'string' && isValidAgentType(a.type) ? a.type : undefined;
     const cleaned: AgentConfig = {
-      url: String(a.url ?? ''),
+      url,
       visibility,
       ...(typeof a.name === 'string' ? { name: a.name } : {}),
-      ...(typeof a.type === 'string' ? { type: a.type as AgentConfig['type'] } : {}),
+      ...(typeValue ? { type: typeValue } : {}),
     };
     return cleaned;
   });
diff --git a/server/tests/unit/agent-visibility-enforcement.test.ts b/server/tests/unit/agent-visibility-enforcement.test.ts
index eb0a2e650e..8c84b8a808 100644
--- a/server/tests/unit/agent-visibility-enforcement.test.ts
+++ b/server/tests/unit/agent-visibility-enforcement.test.ts
@@ -1,10 +1,12 @@
 import { describe, it, expect, vi, beforeEach } from 'vitest';
 import type { AgentConfig } from '../../src/types.js';
 
-// `demotePublicAgentsOnTierDowngrade` now uses `getPool()` + raw SQL with
-// `SELECT … FOR UPDATE` so the read and write can't interleave with a
-// concurrent publish. The mocks below emulate the client contract that
-// the service depends on: BEGIN / SELECT / UPDATE / COMMIT / ROLLBACK.
+/**
+ * `demotePublicAgentsOnTierDowngrade` uses `getPool()` + raw SQL with
+ * `SELECT … FOR UPDATE` so the read and write can't interleave with a
+ * concurrent publish. The mocks below emulate the client contract that
+ * the service depends on: BEGIN / SELECT / UPDATE / COMMIT / ROLLBACK.
+ */
 vi.mock('../../src/db/client.js', () => {
   const release = vi.fn();
   const client: { query: ReturnType<typeof vi.fn>; release: typeof release } = {
@@ -31,16 +33,20 @@ type SelectRow = {
   primary_brand_domain: string | null;
 };
 
+type RecordedQuery = { sql: string; params: unknown[] };
+
 /**
  * Wire the pg client mock so the tx flow (BEGIN → SELECT FOR UPDATE →
- * UPDATE → COMMIT) behaves as described, and capture the UPDATE args
- * for assertion.
+ * UPDATE → COMMIT) behaves as described. Records all queries so tests
+ * can assert the exact SQL + params, not just that *some* UPDATE ran.
  */
-function mockProfileTx(rows: SelectRow[]): { updateArgs: unknown[][] } {
+function mockProfileTx(rows: SelectRow[]): { recorded: RecordedQuery[]; updateArgs: unknown[][] } {
+  const recorded: RecordedQuery[] = [];
   const updateArgs: unknown[][] = [];
   __client.query.mockReset();
   __client.release.mockReset();
   __client.query.mockImplementation(async (sql: string, params?: unknown[]) => {
+    recorded.push({ sql, params: params ?? [] });
     if (sql === 'BEGIN' || sql === 'COMMIT' || sql === 'ROLLBACK') {
       return { rowCount: 0, rows: [] };
     }
@@ -53,18 +59,13 @@ function mockProfileTx(rows: SelectRow[]): { updateArgs: unknown[][] } {
     }
     throw new Error(`Unexpected query: ${sql}`);
   });
-  return { updateArgs };
+  return { recorded, updateArgs };
 }
 
 describe('demotePublicAgentsOnTierDowngrade', () => {
-  let memberDb: any;
   let brandDb: any;
 
   beforeEach(() => {
-    memberDb = {
-      getProfileByOrgId: vi.fn(),
-      updateProfileByOrgId: vi.fn(),
-    };
     brandDb = {
       getDiscoveredBrandByDomain: vi.fn(),
       updateManifestAgents: vi.fn().mockResolvedValue(undefined),
@@ -78,7 +79,7 @@ describe('demotePublicAgentsOnTierDowngrade', () => {
 
   it('no-op when old tier had no API access', async () => {
     const result = await demotePublicAgentsOnTierDowngrade(
-      'org1', 'individual_academic', null, memberDb, brandDb,
+      'org1', 'individual_academic', null, brandDb,
     );
     expect(result).toBeNull();
     expect(__connect).not.toHaveBeenCalled();
@@ -86,35 +87,48 @@ describe('demotePublicAgentsOnTierDowngrade', () => {
 
   it('no-op when new tier still has API access', async () => {
     const result = await demotePublicAgentsOnTierDowngrade(
-      'org1', 'company_icl', 'individual_professional', memberDb, brandDb,
+      'org1', 'company_icl', 'individual_professional', brandDb,
     );
     expect(result).toBeNull();
     expect(__connect).not.toHaveBeenCalled();
   });
 
+  it('locks the profile row with SELECT FOR UPDATE on the supplied orgId', async () => {
+    // Mechanism test — regressions that drop FOR UPDATE or pass the
+    // wrong org param are the exact thing this PR's transactional
+    // rewrite is meant to prevent.
+    const { recorded } = mockProfileTx([
+      { id: 'p1', agents: [agent('https://a', 'public')], primary_brand_domain: null },
+    ]);
+    await demotePublicAgentsOnTierDowngrade('org-target', 'individual_professional', null, brandDb);
+    const selectProfile = recorded.find(r => r.sql.includes('SELECT id, agents, primary_brand_domain'));
+    expect(selectProfile, 'should issue a SELECT for the profile row').toBeTruthy();
+    expect(selectProfile!.sql).toMatch(/FOR UPDATE/);
+    expect(selectProfile!.params[0]).toBe('org-target');
+  });
+
   it('no-op when org has no profile', async () => {
-    mockProfileTx([]);
+    const { recorded } = mockProfileTx([]);
     const result = await demotePublicAgentsOnTierDowngrade(
-      'org1', 'individual_professional', null, memberDb, brandDb,
+      'org1', 'individual_professional', null, brandDb,
     );
     expect(result).toBeNull();
-    // Should open a tx, see 0 rows, ROLLBACK, not UPDATE
-    const queries = __client.query.mock.calls.map((c: any[]) => c[0]);
-    expect(queries).toContain('BEGIN');
-    expect(queries).toContain('ROLLBACK');
-    expect(queries.some((q: string) => q.startsWith?.('UPDATE'))).toBe(false);
+    const sqls = recorded.map(r => r.sql);
+    expect(sqls).toContain('BEGIN');
+    expect(sqls).toContain('ROLLBACK');
+    expect(sqls.some(q => q.trim().startsWith('UPDATE'))).toBe(false);
   });
 
   it('no-op when profile has no public agents', async () => {
     const agents = [agent('https://a.example', 'private'), agent('https://b.example', 'members_only')];
-    mockProfileTx([{ id: 'p1', agents, primary_brand_domain: null }]);
+    const { recorded } = mockProfileTx([{ id: 'p1', agents, primary_brand_domain: null }]);
     const result = await demotePublicAgentsOnTierDowngrade(
-      'org1', 'individual_professional', 'individual_academic', memberDb, brandDb,
+      'org1', 'individual_professional', 'individual_academic', brandDb,
     );
     expect(result).toBeNull();
-    const queries = __client.query.mock.calls.map((c: any[]) => c[0]);
-    expect(queries).toContain('ROLLBACK');
-    expect(queries.some((q: string) => q.trim?.().startsWith('UPDATE member_profiles'))).toBe(false);
+    const sqls = recorded.map(r => r.sql);
+    expect(sqls).toContain('ROLLBACK');
+    expect(sqls.some(q => q.trim().startsWith('UPDATE member_profiles'))).toBe(false);
   });
 
   it('demotes public agents to members_only on Professional → Explorer', async () => {
@@ -127,11 +141,10 @@ describe('demotePublicAgentsOnTierDowngrade', () => {
       { id: 'p1', agents, primary_brand_domain: null },
     ]);
     const result = await demotePublicAgentsOnTierDowngrade(
-      'org1', 'individual_professional', 'individual_academic', memberDb, brandDb,
+      'org1', 'individual_professional', 'individual_academic', brandDb,
     );
     expect(result).toEqual({ orgId: 'org1', demotedCount: 1, brandJsonCleared: false });
     expect(updateArgs).toHaveLength(1);
-    // First UPDATE param is the new agents JSON; assert the public entry flipped.
     const writtenAgents = JSON.parse(updateArgs[0][0] as string);
     expect(writtenAgents).toEqual([
       agent('https://pub.example', 'members_only'),
@@ -145,7 +158,7 @@ describe('demotePublicAgentsOnTierDowngrade', () => {
       { id: 'p1', agents: [agent('https://p.example', 'public')], primary_brand_domain: null },
     ]);
     const result = await demotePublicAgentsOnTierDowngrade(
-      'org1', 'company_leader', null, memberDb, brandDb,
+      'org1', 'company_leader', null, brandDb,
     );
     expect(result?.demotedCount).toBe(1);
   });
@@ -164,7 +177,7 @@ describe('demotePublicAgentsOnTierDowngrade', () => {
       },
     });
     const result = await demotePublicAgentsOnTierDowngrade(
-      'org1', 'individual_professional', null, memberDb, brandDb,
+      'org1', 'individual_professional', null, brandDb,
     );
     expect(result?.brandJsonCleared).toBe(true);
     expect(brandDb.updateManifestAgents).toHaveBeenCalledWith(
@@ -185,14 +198,14 @@ describe('demotePublicAgentsOnTierDowngrade', () => {
       },
     });
     const result = await demotePublicAgentsOnTierDowngrade(
-      'org1', 'individual_professional', null, memberDb, brandDb,
+      'org1', 'individual_professional', null, brandDb,
     );
     expect(result?.brandJsonCleared).toBe(false);
     expect(brandDb.updateManifestAgents).not.toHaveBeenCalled();
   });
 
   it('commits the profile tx before touching brand.json (so a failed manifest write does not orphan the JSONB update)', async () => {
-    mockProfileTx([
+    const { recorded } = mockProfileTx([
       { id: 'p1', agents: [agent('https://p.example', 'public')], primary_brand_domain: 'acme.example' },
     ]);
     brandDb.getDiscoveredBrandByDomain.mockResolvedValue({
@@ -200,15 +213,12 @@ describe('demotePublicAgentsOnTierDowngrade', () => {
       brand_manifest: { agents: [{ url: 'https://p.example', type: 'brand', id: 'p' }] },
     });
     await demotePublicAgentsOnTierDowngrade(
-      'org1', 'individual_professional', null, memberDb, brandDb,
+      'org1', 'individual_professional', null, brandDb,
     );
-    const queries = __client.query.mock.calls.map((c: any[]) => c[0]);
-    const commitAt = queries.indexOf('COMMIT');
+    const sqls = recorded.map(r => r.sql);
+    const commitAt = sqls.indexOf('COMMIT');
     expect(commitAt).toBeGreaterThan(-1);
-    // brand.json write happens after client.release() via the pool, so by
-    // design it can't be in __client.query.mock.calls — no need to assert
-    // ordering there. Instead, verify UPDATE ran before COMMIT.
-    const updateAt = queries.findIndex((q: string) => q.trim?.().startsWith('UPDATE member_profiles'));
+    const updateAt = sqls.findIndex(q => q.trim().startsWith('UPDATE member_profiles'));
     expect(updateAt).toBeLessThan(commitAt);
   });
 });

From e653fca50849dd190dbecb700ece57b10cee6942 Mon Sep 17 00:00:00 2001
From: Brian O'Kelley <bokelley@scope3.com>
Date: Wed, 22 Apr 2026 00:42:27 -0400
Subject: [PATCH 3/3] test: silence CodeQL property-access warnings on mocked
 pg client
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

CodeQL was flagging every read of `__client` / `__connect` (imported
through `vi.mock` via a `@ts-expect-error` shim) as "property access
on null or undefined" — the mock factory always provides them, but
the static analyzer sees unknown/untyped imports.

Replace the bare imports with a validated module-level binding:
reach into the mocked module once, assert the expected shape, and
hand the rest of the file concretely-typed locals (`mockedClient`,
`mockedConnect`). Fails loudly with a descriptive error if the mock
factory ever drifts, which is a test-setup bug we'd want to surface
immediately rather than silently degrade into nullish reads.

Same behavior, same test coverage (10 scenarios pass). No `?.`
noise sprinkled across the assertion bodies.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
---
 .../unit/agent-visibility-enforcement.test.ts | 47 +++++++++++++++----
 1 file changed, 39 insertions(+), 8 deletions(-)

diff --git a/server/tests/unit/agent-visibility-enforcement.test.ts b/server/tests/unit/agent-visibility-enforcement.test.ts
index 8c84b8a808..35a9b41c42 100644
--- a/server/tests/unit/agent-visibility-enforcement.test.ts
+++ b/server/tests/unit/agent-visibility-enforcement.test.ts
@@ -24,8 +24,39 @@ vi.mock('../../src/db/client.js', () => {
 });
 
 import { demotePublicAgentsOnTierDowngrade } from '../../src/services/agent-visibility-enforcement.js';
-// @ts-expect-error — internal test accessors exposed by the vi.mock above
-import { __client, __connect } from '../../src/db/client.js';
+import * as dbClient from '../../src/db/client.js';
+
+/**
+ * Validated module-level bindings for the mock accessors. The mock
+ * factory above always exports these, but the real `db/client.js`
+ * module type doesn't describe them — so we reach in, validate the
+ * shape once, and hand TypeScript (and CodeQL) a concretely-typed
+ * binding to use everywhere else. Throws loudly if the mock plumbing
+ * ever drifts, which is a test-setup bug and should not silently
+ * degrade into nullish property access at runtime.
+ */
+interface MockedPgClient {
+  query: ReturnType<typeof vi.fn>;
+  release: ReturnType<typeof vi.fn>;
+}
+const mockedModule = dbClient as unknown as {
+  __client?: MockedPgClient;
+  __connect?: ReturnType<typeof vi.fn>;
+};
+const mockedClient: MockedPgClient = (() => {
+  const c = mockedModule.__client;
+  if (!c || typeof c.query?.mockReset !== 'function' || typeof c.release?.mockReset !== 'function') {
+    throw new Error('Mocked pg client not wired — vi.mock factory drifted.');
+  }
+  return c;
+})();
+const mockedConnect: ReturnType<typeof vi.fn> = (() => {
+  const c = mockedModule.__connect;
+  if (!c || typeof c.mockClear !== 'function') {
+    throw new Error('Mocked getPool connect not wired — vi.mock factory drifted.');
+  }
+  return c;
+})();
 
 type SelectRow = {
   id: string;
@@ -43,9 +74,9 @@ type RecordedQuery = { sql: string; params: unknown[] };
 function mockProfileTx(rows: SelectRow[]): { recorded: RecordedQuery[]; updateArgs: unknown[][] } {
   const recorded: RecordedQuery[] = [];
   const updateArgs: unknown[][] = [];
-  __client.query.mockReset();
-  __client.release.mockReset();
-  __client.query.mockImplementation(async (sql: string, params?: unknown[]) => {
+  mockedClient.query.mockReset();
+  mockedClient.release.mockReset();
+  mockedClient.query.mockImplementation(async (sql: string, params?: unknown[]) => {
     recorded.push({ sql, params: params ?? [] });
     if (sql === 'BEGIN' || sql === 'COMMIT' || sql === 'ROLLBACK') {
       return { rowCount: 0, rows: [] };
@@ -70,7 +101,7 @@ describe('demotePublicAgentsOnTierDowngrade', () => {
       getDiscoveredBrandByDomain: vi.fn(),
       updateManifestAgents: vi.fn().mockResolvedValue(undefined),
     };
-    __connect.mockClear();
+    mockedConnect.mockClear();
   });
 
   function agent(url: string, visibility: AgentConfig['visibility']): AgentConfig {
@@ -82,7 +113,7 @@ describe('demotePublicAgentsOnTierDowngrade', () => {
       'org1', 'individual_academic', null, brandDb,
     );
     expect(result).toBeNull();
-    expect(__connect).not.toHaveBeenCalled();
+    expect(mockedConnect).not.toHaveBeenCalled();
   });
 
   it('no-op when new tier still has API access', async () => {
@@ -90,7 +121,7 @@ describe('demotePublicAgentsOnTierDowngrade', () => {
       'org1', 'company_icl', 'individual_professional', brandDb,
     );
     expect(result).toBeNull();
-    expect(__connect).not.toHaveBeenCalled();
+    expect(mockedConnect).not.toHaveBeenCalled();
   });
 
   it('locks the profile row with SELECT FOR UPDATE on the supplied orgId', async () => {