fix(graphile-llm): key agent discovery by database_id, and actually run express-context in CI

.github/workflows/run-tests.yaml

@@ -99,7 +99,7 @@ jobs:
           - batch: uploads
             packages: 'uploads/mime-bytes uploads/uuid-hash uploads/uuid-stream uploads/etag-hash uploads/etag-stream uploads/stream-to-etag uploads/content-type-stream uploads/upload-names'
           - batch: packages-core
-            packages: 'packages/url-domains postgres/query-builder packages/csrf packages/oauth packages/12factor-env packages/orm'
+            packages: 'packages/url-domains postgres/query-builder packages/csrf packages/oauth packages/12factor-env packages/orm packages/express-context'
           - batch: packages-services
             packages: 'packages/postmaster packages/smtppostmaster packages/csv-to-pg packages/cli postgres/pgsql-client postgres/pg-ast'
           - batch: graphql

graphile/graphile-llm/__tests__/agent-discovery.test.ts

@@ -0,0 +1,89 @@
+import type { Pool } from 'pg';
+
+import {
+  clearAgentDiscoveryCache,
+  getAgentDiscovery
+} from '../src/plugins/agent-discovery-plugin';
+
+const DB_A = '00000000-0000-0000-0000-00000000000a';
+const DB_B = '00000000-0000-0000-0000-00000000000b';
+
+const row = (prefix: string): Record<string, string | null> => ({
+  schema_name: `${prefix}_agent_public`,
+  thread_table_name: 'agent_thread',
+  message_table_name: 'agent_message',
+  task_table_name: null
+});
+
+interface Call {
+  text: string;
+  values?: unknown[];
+}
+
+const fakePool = (respond: (values: unknown[]) => { rows: unknown[] }) => {
+  const calls: Call[] = [];
+  const pool = {
+    query: jest.fn(async (text: string, values?: unknown[]) => {
+      calls.push({ text, values });
+      return respond(values ?? []);
+    })
+  } as unknown as Pool;
+  return { pool, calls };
+};
+
+const pgError = (code: string) => Object.assign(new Error(`pg error ${code}`), { code });
+
+beforeEach(() => clearAgentDiscoveryCache());
+
+describe('getAgentDiscovery', () => {
+  it('resolves each tenant its own agent tables', async () => {
+    // The unkeyed query this replaced returned the same row to both, and the
+    // per-database cache then made the wrong answer stick for its TTL.
+    const { pool, calls } = fakePool(values => ({
+      rows: [row(values[0] === DB_A ? 'a' : 'b')]
+    }));
+
+    const a = await getAgentDiscovery(pool, DB_A);
+    const b = await getAgentDiscovery(pool, DB_B);
+
+    expect(calls.map(c => c.values)).toEqual([[DB_A], [DB_B]]);
+    expect(calls[0].text).toMatch(/WHERE acm\.database_id = \$1/);
+    expect(a?.thread?.schemaName).toBe('a_agent_public');
+    expect(b?.thread?.schemaName).toBe('b_agent_public');
+  });
+
+  it('caches per database id, not across databases', async () => {
+    const { pool, calls } = fakePool(values => ({
+      rows: [row(values[0] === DB_A ? 'a' : 'b')]
+    }));
+
+    await getAgentDiscovery(pool, DB_A);
+    await getAgentDiscovery(pool, DB_A);
+    expect(calls).toHaveLength(1);
+
+    await getAgentDiscovery(pool, DB_B);
+    expect(calls).toHaveLength(2);
+  });
+
+  it('treats an absent module as not provisioned', async () => {
+    const { pool } = fakePool(() => {
+      throw pgError('42P01');
+    });
+    await expect(getAgentDiscovery(pool, DB_A)).resolves.toBeNull();
+  });
+
+  it('rethrows anything that is not the absence it probes for', async () => {
+    // A dead pool reported as "not provisioned" is an API that silently loses
+    // its agent surface.
+    const { pool } = fakePool(() => {
+      throw pgError('57P01'); // admin_shutdown
+    });
+    await expect(getAgentDiscovery(pool, DB_A)).rejects.toThrow(/57P01/);
+  });
+
+  it('refuses a missing databaseId rather than querying unkeyed', async () => {
+    const { pool, calls } = fakePool(() => ({ rows: [] }));
+    await expect(getAgentDiscovery(pool, '')).rejects.toThrow(/databaseId is required/);
+    expect(calls).toHaveLength(0);
+  });
+});

graphile/graphile-llm/src/plugins/agent-discovery-plugin.ts

@@ -7,6 +7,11 @@
  *
  * Results are cached per-database with a TTL so the REST middleware
  * doesn't hit the database on every request.
+ *
+ * Discovery is keyed by `database_id`, as every other module lookup is: one
+ * serving database holds several tenants' schemas, so an unkeyed lookup does
+ * not fail — it resolves a neighbouring tenant's agent tables, and the cache
+ * then serves that for its whole TTL.
  */
 
 import { ModuleConfigCache } from 'graphile-cache';
@@ -49,26 +54,44 @@ const DISCOVERY_SQL = `
     acm.task_table_name
   FROM metaschema_modules_public.agent_chat_module acm
   JOIN metaschema_public.schema s ON s.id = acm.schema_id
+  WHERE acm.database_id = $1
   LIMIT 1
 `;
 
+/** The module (or the whole metaschema) is simply absent from this database. */
+const NOT_PROVISIONED = new Set([
+  '42P01', // undefined_table
+  '3F000' // invalid_schema_name
+]);
+
+const isNotProvisioned = (err: unknown): boolean =>
+  typeof err === 'object' &&
+  err !== null &&
+  'code' in err &&
+  typeof err.code === 'string' &&
+  NOT_PROVISIONED.has(err.code);
+
 /**
  * Look up agent table info for a database, querying the module config table.
- * Results are cached per-database with a 60s TTL.
+ * Results are cached per database id with a 60s TTL.
  */
 export async function getAgentDiscovery(
   pool: Pool,
-  dbname: string
+  databaseId: string
 ): Promise<AgentDiscovery | null> {
-  const cached = agentDiscoveryCache.get(dbname);
+  if (!databaseId) {
+    throw new Error('getAgentDiscovery: databaseId is required');
+  }
+
+  const cached = agentDiscoveryCache.get(databaseId);
   if (cached !== undefined) {
     return cached;
   }
 
   let discovery: AgentDiscovery | null = null;
 
   try {
-    const { rows } = await pool.query(DISCOVERY_SQL);
+    const { rows } = await pool.query(DISCOVERY_SQL, [databaseId]);
 
     if (rows.length > 0) {
       const row = rows[0];
@@ -86,10 +109,12 @@ export async function getAgentDiscovery(
           : null
       };
     }
-  } catch {
-    // Module table doesn't exist in this database — not provisioned
+  } catch (err) {
+    // Only the absence being probed for is swallowed. A dead pool or a bad
+    // databaseId reported as "not provisioned" is a silently agent-less API.
+    if (!isNotProvisioned(err)) throw err;
   }
 
-  agentDiscoveryCache.set(dbname, discovery);
+  agentDiscoveryCache.set(databaseId, discovery);
   return discovery;
 }