fix(connectors): declare Discord's readiness so the catalogue can list it (TASK-024)

backend/__tests__/unit/integrations/manifestReadiness.test.js

@@ -1,6 +1,10 @@
 const { manifests } = require('../../../integrations/manifests');
+const { providerReadiness } = require('../../../services/installable/installableCatalogService');
 
 const READINESS_ENV = [
+  'DISCORD_BOT_TOKEN',
+  'DISCORD_CLIENT_ID',
+  'DISCORD_CLIENT_SECRET',
   'TELEGRAM_BOT_TOKEN',
   'TELEGRAM_SECRET_TOKEN',
   'TELEGRAM_WEBHOOK_ALLOW_UNVERIFIED',
@@ -64,9 +68,50 @@ describe('installable connector manifest readiness', () => {
   it('describes Telegram as a chat the pods the member turns on reach (TASK-154)', () => {
     expect(manifests.telegram.catalog.description)
       .toBe('Link your Telegram chat to Commonly — every pod you turn on reaches it.');
+
+    // (This commit's own copy of this test asserted the one-pod sentence TASK-154
+    // reversed; it is dropped here rather than merged, and the guard below is the
+    // one that reads the CURRENT sentence.)
     expect(manifests.telegram.catalog.description).not.toMatch(/ingest|summar/i);
   });
 
+
+  // Discord is a shipping connector (routes/discord.ts; discordProvider.ts). It
+  // was absent from the channel catalog only because it declared no readiness(),
+  // which is the predicate providerInstallableIds() filters on — so the defect
+  // was a missing declaration, not a missing capability. These assert the
+  // consequence through the real service rather than through a re-implementation
+  // of its filter.
+  it('reports Discord as an answerable provider, not an unknown Installable', () => {
+    expect(providerReadiness('discord')).toEqual({ available: false, reason: 'not_configured' });
+
+    process.env.DISCORD_BOT_TOKEN = 'configured';
+    process.env.DISCORD_CLIENT_ID = 'configured';
+    process.env.DISCORD_CLIENT_SECRET = 'configured';
+
+    expect(providerReadiness('discord')).toEqual({ available: true });
+  });
+
+  it('requires all three of the keys the Discord install route reads', () => {
+    process.env.DISCORD_BOT_TOKEN = 'configured';
+    process.env.DISCORD_CLIENT_ID = 'configured';
+    expect(providerReadiness('discord')).toEqual({ available: false, reason: 'not_configured' });
+
+    process.env.DISCORD_CLIENT_SECRET = 'configured';
+    expect(providerReadiness('discord')).toEqual({ available: true });
+
+    process.env.DISCORD_CLIENT_SECRET = '   ';
+    expect(providerReadiness('discord')).toEqual({ available: false, reason: 'not_configured' });
+  });
+
+  it('leaves groupme undeclared, so this change cannot silently widen the catalog', () => {
+    process.env.DISCORD_BOT_TOKEN = 'configured';
+    process.env.DISCORD_CLIENT_ID = 'configured';
+    process.env.DISCORD_CLIENT_SECRET = 'configured';
+
+    expect(providerReadiness('groupme')).toBeNull();
+  });
+
   // Its own arm, because the copy this row used to carry taught one chat = one
   // pod, and TASK-154's ruling reversed that: one private chat binds one member,
   // and their pods sit behind it as gates. A separate test is what makes the

backend/__tests__/unit/routes/installables.test.js

@@ -16,6 +16,7 @@ jest.mock('../../../models/Pod', () => ({ findById: jest.fn() }));
 jest.mock('../../../services/installable/installableCatalogService', () => ({
   catalogFor: jest.fn(),
   providerReadiness: jest.fn(() => ({ available: true })),
+  providerOffered: jest.fn(() => true),
   publicIntegration: jest.fn((integration) => {
     if (!integration?.config?.adminPause) return integration;
     return {
@@ -67,6 +68,7 @@ describe('installable connector routes', () => {
   beforeEach(() => {
     jest.clearAllMocks();
     catalogService.providerReadiness.mockReturnValue({ available: true });
+    catalogService.providerOffered.mockResolvedValue(true);
   });
 
   it('requires auth for the user-scoped catalog', async () => {
@@ -109,6 +111,46 @@ describe('installable connector routes', () => {
     expect(installationService.install).not.toHaveBeenCalled();
   });
 
+  it('refuses a usable provider this instance does not offer, before the pod lookup', async () => {
+    // Capability is not offerability (Wren 71174). This is the request a stranger
+    // made after the page showed them an Add for Discord: readiness passed, and
+    // the install then died further in as installable_not_found. Refused at the
+    // same place as the missing-credential case, so the API cannot be driven into
+    // a state the UI will not offer.
+    catalogService.providerReadiness.mockReturnValue({ available: true });
+    catalogService.providerOffered.mockResolvedValue(false);
+
+    const res = await request(app)
+      .post('/api/installables/discord/install')
+      .set(auth)
+      .send({ podId });
+
+    expect(res.status).toBe(422);
+    expect(res.body).toEqual({ code: 'provider_not_offered' });
+    expect(Pod.findById).not.toHaveBeenCalled();
+    expect(installationService.install).not.toHaveBeenCalled();
+  });
+
+  it('lets a provider through when it is both usable and offered', async () => {
+    // The paired control: the new guard must not refuse everything. Without this,
+    // a predicate that always returned false would pass the test above.
+    catalogService.providerOffered.mockResolvedValue(true);
+    Pod.findById.mockResolvedValue({
+      _id: podId,
+      createdBy: { toString: () => '64b64c48c4f37a6b2f34c111' },
+      members: ['64b64c48c4f37a6b2f34c111'],
+    });
+    installationService.install.mockResolvedValue({ httpStatus: 201, state: 'installing', installation: null, integration: null });
+
+    const res = await request(app)
+      .post('/api/installables/discord/install')
+      .set(auth)
+      .send({ podId });
+
+    expect(res.status).toBe(201);
+    expect(installationService.install).toHaveBeenCalled();
+  });
+
   it('rejects a non-member before any install row is claimed', async () => {
     Pod.findById.mockResolvedValue({
       _id: podId,

backend/__tests__/unit/services/installableOfferability.test.js

@@ -0,0 +1,130 @@
+// @ts-nocheck
+
+// Offerability: a provider is offered when this instance holds the credentials
+// (readiness) AND a builtin active Installable row carries it (the roster).
+//
+// Why this file exists at all, and why it uses the real store and the real
+// manifests: every other test of the catalogue injects fake stores
+// (`installableCatalogService.test.js` mocks `Installable.find`), so
+// "readiness declared, no row seeded" was UNREPRESENTABLE in the suite. That is
+// the exact state that shipped a lowercase `discord` row whose Add ended in 404
+// installable_not_found (Vera 71152, Wren 71162). A mocked store can only test
+// the catalogue's arithmetic; it cannot test whether the roster agrees with it.
+//
+// @kais 71169 ruling: `catalogFor` keeps one entry per readiness-declaring
+// manifest (nothing vanishes) and reports `offered`; `availableProviders` and
+// the row action both read it.
+
+jest.mock('jsonwebtoken', () => ({}));
+
+const mongoose = require('mongoose');
+
+const Installable = require('../../../models/Installable');
+const { catalogFor, providerOffered } = require('../../../services/installable/installableCatalogService');
+const { TELEGRAM_CONNECTOR } = require('../../../scripts/seed-builtin-connectors');
+const { setupMongoDb, closeMongoDb, clearMongoDb } = require('../../utils/testUtils');
+
+const USER_ID = new mongoose.Types.ObjectId().toString();
+
+const channelsOf = async (userId = USER_ID) => {
+  const { installables } = await catalogFor(userId);
+  return installables.filter((entry) => entry.list === 'channels');
+};
+
+const byId = (entries) => new Map(entries.map((entry) => [entry.installableId, entry]));
+
+describe('provider offerability', () => {
+  beforeAll(async () => { await setupMongoDb(); });
+  afterAll(async () => { await closeMongoDb(); });
+  beforeEach(async () => { await clearMongoDb(); });
+
+  it('reports one channel entry per readiness-declaring manifest, offered or not', async () => {
+    // The invariant that keeps Discord from VANISHING, which is the failure mode
+    // on the other side of this defect: an entry is dropped when a provider has
+    // readiness and no row, so the page stops being able to say anything about it.
+    const { manifests } = require('../../../integrations/manifests');
+    const declared = Object.values(manifests)
+      .filter((manifest) => typeof manifest.readiness === 'function')
+      .map((manifest) => manifest.id)
+      .sort();
+
+    const entries = await channelsOf();
+
+    expect(entries.map((entry) => entry.installableId).sort()).toEqual(declared);
+    expect(new Set(entries.map((entry) => entry.installableId)).size).toBe(entries.length);
+  });
+
+  it('never labels a row with its own raw id', async () => {
+    // The visible half of the bug: `label: installable?.name || installableId`
+    // drew a lowercase `discord` on a page whose every other row is a word.
+    const entries = await channelsOf();
+
+    entries.forEach((entry) => {
+      expect(entry.label).not.toBe(entry.installableId);
+      expect(entry.label).toBeTruthy();
+    });
+    expect(byId(entries).get('discord').label).toBe(manifestsLabel('discord'));
+  });
+
+  it('offers a provider once a builtin active row exists, and not before', async () => {
+    const before = byId(await channelsOf());
+    expect(before.get('telegram').offered).toBe(false);
+    expect(await providerOffered('telegram')).toBe(false);
+
+    await Installable.create({ ...TELEGRAM_CONNECTOR });
+
+    const after = byId(await channelsOf());
+    expect(after.get('telegram').offered).toBe(true);
+    expect(await providerOffered('telegram')).toBe(true);
+    // Offering one provider must not offer its neighbours: a predicate that
+    // answered from the wrong field would flip every row at once.
+    expect(after.get('slack').offered).toBe(false);
+    expect(after.get('discord').offered).toBe(false);
+  });
+
+  it('does not count a row that is not builtin and active', async () => {
+    // The predicate is a join, not a presence check. A marketplace row, or a
+    // retired builtin one, must not make a provider installable: `install()`
+    // resolves the same way and would still answer installable_not_found.
+    await Installable.create({ ...TELEGRAM_CONNECTOR, source: 'marketplace' });
+    expect(await providerOffered('telegram')).toBe(false);
+
+    await Installable.deleteMany({});
+    await Installable.create({ ...TELEGRAM_CONNECTOR, status: 'deprecated' });
+    expect(await providerOffered('telegram')).toBe(false);
+  });
+
+  it('keeps capability and offerability independent', async () => {
+    // Offered without configured is a real state (a seeded row on an instance
+    // with no keys) and so is configured without offered (Discord before this
+    // PR). The page needs both bits to pick the right of three state lines;
+    // collapsing them is what produced a single wrong answer.
+    const original = process.env.DISCORD_BOT_TOKEN;
+    await Installable.create({ ...TELEGRAM_CONNECTOR });
+
+    const sealed = byId(await channelsOf());
+    expect(sealed.get('telegram').offered).toBe(true);
+    expect(sealed.get('discord').offered).toBe(false);
+    expect(sealed.get('discord').available).toBe(false);
+
+    process.env.DISCORD_BOT_TOKEN = 'configured';
+    process.env.DISCORD_CLIENT_ID = 'configured';
+    process.env.DISCORD_CLIENT_SECRET = 'configured';
+    try {
+      const configured = byId(await channelsOf());
+      expect(configured.get('discord').available).toBe(true);
+      expect(configured.get('discord').offered).toBe(false);
+    } finally {
+      delete process.env.DISCORD_CLIENT_ID;
+      delete process.env.DISCORD_CLIENT_SECRET;
+      if (original === undefined) delete process.env.DISCORD_BOT_TOKEN;
+      else process.env.DISCORD_BOT_TOKEN = original;
+    }
+  });
+});
+
+function manifestsLabel(id) {
+  // eslint-disable-next-line global-require
+  const { manifests } = require('../../../integrations/manifests');
+  return manifests[id].catalog.label;
+}

backend/integrations/manifests.ts

@@ -77,6 +77,20 @@ const manifests: Record<string, IntegrationManifest> = {
     // the same route refuses with `server_owned_config_key` (TASK-140).
     requiredConfig: ['serverId', 'channelId', 'botToken'],
     configSchema: buildConfigSchema(['serverId', 'channelId', 'botToken']),
+    // Discord is a shipping connector (routes/discord.ts: OAuth install-link,
+    // callback, binding, uninstall) and discordProvider.ts implements the same
+    // provider interface as slack/telegram — it was absent from the catalog only
+    // because it declared no readiness(), which is what
+    // providerInstallableIds() filters on. Gate on the three secrets that
+    // route actually reads, so the readiness answer tracks the install path
+    // rather than a second list that can drift from it.
+    readiness: () => (
+      hasConfiguration(
+        'DISCORD_BOT_TOKEN',
+        'DISCORD_CLIENT_ID',
+        'DISCORD_CLIENT_SECRET',
+      ) ? { available: true } : notConfigured()
+    ),
     catalog: {
       label: 'Discord',
       provider: 'discord',

backend/routes/installables.ts

@@ -24,6 +24,7 @@ const connectorDeliveryFailures = require('../services/connectorDeliveryFailureS
 // eslint-disable-next-line global-require
 const {
   catalogFor,
+  providerOffered,
   providerReadiness,
   publicIntegration,
 } = require('../services/installable/installableCatalogService');
@@ -567,6 +568,14 @@ router.post('/:installableId/install', writeIntegrationsRateLimit, auth, async (
   if (readiness && !readiness.available) {
     return res.status(422).json({ code: 'provider_not_configured' });
   }
+  // Capability is not offerability (Wren 71174). Without this, a provider the
+  // instance has credentials for but has never rostered passes the readiness
+  // guard and dies further in as `installable_not_found` — the 404 a stranger
+  // met after the page showed them an Add button. Refused here, before the pod
+  // lookup, so the API cannot be driven into a state the UI will not offer.
+  if (!(await providerOffered(installableId))) {
+    return res.status(422).json({ code: 'provider_not_offered' });
+  }
 
   try {
     const pod = await Pod.findById(podId);

backend/services/installable/installableCatalogService.ts

@@ -29,6 +29,33 @@ const providerReadiness = (installableId: string): ProviderReadiness | null => {
   return typeof manifest?.readiness === 'function' ? manifest.readiness() : null;
 };
 
+// Declaring `readiness` is a claim about CAPABILITY; having a builtin active
+// Installable row is the claim about OFFERABILITY. They are different questions
+// and only one of them was being asked, which is how a provider could render as
+// connectable and then die on Add with `installable_not_found` (Vera 71152,
+// Wren 71169). `offeredByRoster` is the single decision; the two readers below
+// differ only in cardinality, because the catalog has the rows in hand and the
+// route has one id.
+const offeredByRoster = (installable: unknown): boolean => Boolean(installable);
+
+const providerLabel = (installableId: string): string => {
+  const manifest = (manifests as Record<string, { catalog?: { label?: string } }>)[installableId];
+  return manifest?.catalog?.label || installableId;
+};
+
+// The route's reader: one id, so one query. Exported because the install route
+// refuses on the same predicate the catalog advertises, and a guard that lives
+// only in the UI cannot stop the API being driven into a state the page will not
+// offer.
+const providerOffered = async (installableId: string): Promise<boolean> => {
+  const installable = await Installable.findOne({
+    installableId,
+    source: 'builtin',
+    status: 'active',
+  }).lean();
+  return offeredByRoster(installable);
+};
+
 // Mongoose's Integration toJSON transform is the normal guard. Lean catalog
 // reads bypass it, so they run the same strip explicitly: one key list, so a
 // credential can never be serialized here that toJSON would have dropped.
@@ -161,9 +188,13 @@ const catalogFor = async (userId: string): Promise<{ installables: unknown[] }>
       return {
         installableId,
         list: 'channels',
-        label: installable?.name || installableId,
+        // The row's name comes from the roster when there is one, and from the
+        // manifest's own catalog block when there is not. Falling back to the
+        // raw id is what put a lowercase `discord` on the page (Wren 71162).
+        label: installable?.name || providerLabel(installableId),
         description: installable?.description || '',
         available: readiness.available,
+        offered: offeredByRoster(installable),
         ...(readiness.reason ? { unavailableReason: readiness.reason } : {}),
         installation: publicInstallation(installation),
         integration: installation
@@ -174,6 +205,6 @@ const catalogFor = async (userId: string): Promise<{ installables: unknown[] }>
   };
 };
 
-module.exports = { catalogFor, providerReadiness, publicIntegration };
+module.exports = { catalogFor, providerReadiness, providerOffered, publicIntegration };
 
 export {};

frontend/src/i18n/locales/en.json

@@ -1925,6 +1925,7 @@
     "newCode": "New code",
     "nextStep": "Next step",
     "noPods": "Join a pod first.",
+    "notConnectable": "Not connectable yet.",
     "notConnected": "not connected",
     "notEnabled": "Not enabled on this instance.",
     "notLinkedDetail": "your messages have nowhere to go",

frontend/src/i18n/locales/zh-CN.json

@@ -1917,6 +1917,7 @@
     "newCode": "新启用码",
     "nextStep": "下一步",
     "noPods": "请先加入一个 Pod。",
+    "notConnectable": "暂不可连接。",
     "notConnected": "未连接",
     "notEnabled": "此实例尚未启用。",
     "notLinkedDetail": "你的消息还没有去处",