diff --git a/.github/workflows/run-tests.yaml b/.github/workflows/run-tests.yaml index 8f23601d7..d842fea40 100644 --- a/.github/workflows/run-tests.yaml +++ b/.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 diff --git a/graphile/graphile-llm/__tests__/agent-discovery.test.ts b/graphile/graphile-llm/__tests__/agent-discovery.test.ts new file mode 100644 index 000000000..0ed0b30a5 --- /dev/null +++ b/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 => ({ + 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); + }); +}); diff --git a/graphile/graphile-llm/src/plugins/agent-discovery-plugin.ts b/graphile/graphile-llm/src/plugins/agent-discovery-plugin.ts index ad020c4dc..15347c94d 100644 --- a/graphile/graphile-llm/src/plugins/agent-discovery-plugin.ts +++ b/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,18 +54,36 @@ 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 { - const cached = agentDiscoveryCache.get(dbname); + if (!databaseId) { + throw new Error('getAgentDiscovery: databaseId is required'); + } + + const cached = agentDiscoveryCache.get(databaseId); if (cached !== undefined) { return cached; } @@ -68,7 +91,7 @@ export async function getAgentDiscovery( 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; }