diff --git a/docs/AGENT.md b/docs/AGENT.md index ea1d803e..e58845f7 100644 --- a/docs/AGENT.md +++ b/docs/AGENT.md @@ -2501,12 +2501,6 @@ the role's own grants are the whole boundary (A3). and never what it said, so an empty completion reaches it too and a model's recorded `retryEmptyTurn: false` decides nothing. Pinned as it behaves rather than narrowed, because the narrowing would move behaviour five passing runs were measured under. -- **B76** — the aggregated capture that fixed the wide-catalog refusal now ADMITS the image's own - extension views, so a grounded run on TimescaleDB, Cloudberry or AlloyDB Omni reasons over an - inventory that is mostly internal objects (measured: AlloyDB's least-privilege role sees 67 extension - views beside its 2 user tables). Not a privilege leak — the role genuinely sees them — but grounding - noise; the object set was the same under the flat projection, which refused before any of it reached - a run. **Settled as limits rather than as work.** The seven below have no entry in `docs/BACKLOG.md`, and that is the point: each is how the product behaves, stated where a reader of this document will meet diff --git a/docs/BACKLOG.md b/docs/BACKLOG.md index b860ec32..a8b6acc0 100644 --- a/docs/BACKLOG.md +++ b/docs/BACKLOG.md @@ -40,7 +40,7 @@ None of it is a GitHub issue. - [Security Phase 2 deferrals](#security-phase-2-deferrals) — C3–C11 · 7 - [Security Phase 3 deferrals](#security-phase-3-deferrals) — K4 - [Agent M1 deferrals (#328)](#agent-m1-deferrals-328) — A1–A5 · 4 -- [Agent M2 deferrals (#329)](#agent-m2-deferrals-329) — B2–B76 · 22 +- [Agent M2 deferrals (#329)](#agent-m2-deferrals-329) — B2–B75 · 21 --- @@ -1835,31 +1835,3 @@ so), and none of them has been measured. **Done when:** a resume onto a repointed connection does one stated thing, and the run's own record says which. - -### B76. The aggregated grounding capture admits the image's own extension views - -The aggregation that fixed the row-cap refusal made a stock image's wide catalog capture SUCCEED instead -of refusing, and the price -is that it succeeds by admitting the image's own objects. Measured live on 2026-09-01 against the -`compat` profile images (`database-compose.yml`), with two user tables seeded: - -- TimescaleDB (`timescale/timescaledb:latest-pg17`): 385 column rows aggregate to 46 tables. -- Cloudberry (`woblerr/cloudberry:2.1.0-incubating`): 479 to 67 as gpadmin, 53 as a least-privilege role. -- AlloyDB Omni (`google/alloydbomni:17.9.0`): 542 to 70 as postgres, 69 as a least-privilege role. - -AlloyDB is the sharp case: the least-privilege role's 69 tables are 2 user tables plus 67 extension views, -and where they SIT is the part that makes them noise rather than a schema to exclude — 49 of them are -installed into `public` itself (`google_db_advisor_*`, `hypopg_list_indexes`), beside 14 in `google_ml` and -4 in `ai`. Not a privilege leak — -`information_schema.columns` applies its own visibility rules and the role genuinely sees those views — -but grounding noise: the inventory a run reasons over is mostly objects the user did not create, and the -model-facing pack spends `AGENT_CONTEXT_PACK_MAX_CHARS` ranking them against the objective. The old flat -projection had the same object set; it just refused before any of it reached a run. - -The candidate fixes are the ones the row-cap fix recorded and set aside: filter by `table_type`, or -exclude the objects the object browser already treats as internal. That fix refuted only the -schema-exclusion variant AS A FIX FOR THE ROW CAP; as a fix for grounding noise the question is open and -unmeasured. - -**Done when:** a run grounded on one of these images reasons over the user's objects — or the internal -ones are excluded — with a test per shape (TimescaleDB, Cloudberry, AlloyDB Omni). diff --git a/docs/providers/postgres.md b/docs/providers/postgres.md index e797ec25..42f5e30a 100644 --- a/docs/providers/postgres.md +++ b/docs/providers/postgres.md @@ -1083,12 +1083,20 @@ path is reachable from server code and not from a request. Four things about the PostgreSQL side of that layer are worth knowing here: - **The catalog read is a composed bounded read**, not a new operation. `inspect_schema` takes a - schema/table selector and the server writes - `SELECT … FROM information_schema.columns WHERE table_schema NOT IN ('pg_catalog', …)`, executed as - `sql.query.read` like any other statement. The model never supplies that SQL. Selectors are quoted - with `quoteLiteral` because `queryReadOnly` binds no parameters, and a selector carrying a - backslash is refused outright rather than quoted — the dialect-less span reader treats it as an - escape, so `'a\'` would read as an unterminated literal. + schema/table selector and the server writes the `columns` statement itself, executed as + `sql.query.read` like any other statement. The model never supplies that SQL. The statement's + `WHERE` excludes the engine's own objects three ways: the full engine-builtin schema list (not + only `pg_catalog` / `information_schema`) and every schema an extension created (`pg_depend` on + `pg_namespace`, `deptype = 'e'`) — both copied from this provider's object browser — plus every + relation an extension created (`pg_depend` on `pg_class`, `deptype = 'e'`), which is new on the + agent path and carried by all four catalog reads, not only the column one. The relation test is + the one that reaches AlloyDB Omni's extension views, 49 of which sit in `public` itself where no + schema filter can reach them; a user's own views are never extension-owned, so they stay in the + inventory. Measured live on 2026-09-07 with two user tables seeded: 46 → 2 object rows on + TimescaleDB, 67 → 2 on Cloudberry and 70 → 2 on AlloyDB Omni. + Selectors are quoted with `quoteLiteral` because `queryReadOnly` binds no parameters, and a selector + carrying a backslash is refused outright rather than quoted — the dialect-less span reader treats it + as an escape, so `'a\'` would read as an unterminated literal. - **A run reads three catalog inventories at its start (#329 T8), not one.** `inspect_schema` takes a `kind` — `columns` (the default), `relations` (foreign keys, from `pg_constraint` with `unnest(conkey, confkey) WITH ORDINALITY` pairing the two sides) and `indexes` (from `pg_index` diff --git a/src/lib/agent/composed-sql.ts b/src/lib/agent/composed-sql.ts index 2f2870a7..75bb2ee8 100644 --- a/src/lib/agent/composed-sql.ts +++ b/src/lib/agent/composed-sql.ts @@ -154,6 +154,119 @@ function equalsClause(column: string, value: string | undefined, field: string, return ` AND ${column} = ${quoteLiteral(assertSelector(value, field), dialect)}`; } +// ─── PostgreSQL internal-object exclusion ───────────────────────────────── + +/** + * The engine-builtin schemas a PostgreSQL grounding read must not treat as user + * data. This is the SAME set the provider's object browser uses + * (`SYSTEM_SCHEMAS` in `src/lib/db/providers/sql/postgres.ts`), copied rather + * than imported because the agent side must not depend on the provider module. + * `tests/unit/lib/agent/composed-sql.test.ts` pins the two lists together so + * they cannot drift. Each group is sorted by engine and checked against that + * engine's own documentation or, where the doc is silent, a live instance. + */ +const POSTGRES_SYSTEM_SCHEMAS = [ + // PostgreSQL itself. + "pg_catalog", + "information_schema", + "pg_toast", + // Materialize - materialize.com/docs/sql/system-catalog/ + "mz_catalog", + "mz_internal", + "mz_introspection", + // CockroachDB - cockroachlabs.com/docs/stable/system-catalogs enumerates exactly + // four schemas; the two below are the ones stock PostgreSQL does not also have. + "crdb_internal", + "pg_extension", + // TimescaleDB - the extension's own sql/pre_install/schemas.sql creates all seven. + // `_timescaledb_internal` is the one that floods: it holds every hypertable chunk. + "_timescaledb_catalog", + "_timescaledb_config", + "_timescaledb_functions", + "_timescaledb_internal", + "_timescaledb_cache", + "timescaledb_experimental", + "timescaledb_information", + // Apache Cloudberry - cloudberry.apache.org create-and-manage-schemas documents the + // first three. `pg_ext_aux` is not in that page but holds the PAX auxiliary tables + // (pg_pax_tables, pg_pax_fastsequence) on a live 2.1.0 instance, so it is here on + // measurement rather than on the doc's authority. + "gp_toolkit", + "pg_aoseg", + "pg_bitmapindex", + "pg_ext_aux", +] as const; + +// Rendered once. Callers interpolate this into a `NOT IN (...)` clause. +const POSTGRES_SYSTEM_SCHEMA_LIST = POSTGRES_SYSTEM_SCHEMAS.map((schema) => `'${schema}'`).join(", "); + +/** + * Schemas an EXTENSION created. AlloyDB Omni is deliberately absent from the + * name list above: its `google_ml` and `ai` schemas are extension-created, and + * those are names a user could plausibly choose for schemas of their own, so + * hiding them by name would make the user's tables vanish. Ownership is the + * question the name was standing in for, and `pg_depend` answers it directly. + * Same query the provider composes (`EXTENSION_OWNED_SCHEMAS_SQL`). + */ +const POSTGRES_EXTENSION_OWNED_SCHEMAS_SQL = + "SELECT n.nspname FROM pg_namespace n " + + "JOIN pg_depend d ON d.objid = n.oid AND d.classid = 'pg_namespace'::regclass AND d.deptype = 'e' " + + "JOIN pg_extension e ON e.oid = d.refobjid"; + +/** + * RELATIONS (tables, views, materialized views, sequences) an extension created + * — the B76 sharp case. AlloyDB Omni installs 49 of its 67 extension views into + * `public` itself, where no schema filter can reach them, so the OBJECT must be + * excluded by ownership rather than by where it sits. This is the same + * `pg_depend` ownership test with `classid = 'pg_class'::regclass`: it returns + * the extension's own relations and nothing a user created, so a user's own + * views survive. + */ +const POSTGRES_EXTENSION_OWNED_RELATIONS_SQL = + "SELECT n.nspname, c.relname FROM pg_class c " + + "JOIN pg_namespace n ON n.oid = c.relnamespace " + + "JOIN pg_depend d ON d.objid = c.oid AND d.classid = 'pg_class'::regclass AND d.deptype = 'e' " + + "JOIN pg_extension e ON e.oid = d.refobjid"; + +/** + * The full "this schema is not the engine's own" test for one column: a fixed + * list of engine-builtin schemas, plus anything an extension created. Mirrors + * the provider's `schemaExclusion(column)`. + */ +function postgresSchemaExclusion(column: string): string { + return `${column} NOT IN (${POSTGRES_SYSTEM_SCHEMA_LIST}) AND ${column} NOT IN (${POSTGRES_EXTENSION_OWNED_SCHEMAS_SQL})`; +} + +/** + * The relation-level ownership test for one (schema, table) pair. Every + * PostgreSQL catalog read carries it, so `inspect_schema` answers the same + * object set whatever `kind` it is asked for: a relation an extension created + * is absent from columns, indexes, statistics and relations alike, and only a + * user's own objects survive. + */ +function postgresRelationExclusion(schemaColumn: string, tableColumn: string): string { + return `(${schemaColumn}, ${tableColumn}) NOT IN (${POSTGRES_EXTENSION_OWNED_RELATIONS_SQL})`; +} + +/** + * The same statement with both ownership tests removed, leaving the fixed + * engine-builtin schema list alone. This is the agent-side mirror of the + * provider's `withoutExtensionOwnershipTest`: the driver serves PostgreSQL-wire + * engines nobody here has run, and one without `pg_depend`/`pg_extension` + * raises on those names, so the read retries with the fixed list alone. Kept as + * a pure string rewrite so the caller decides when to use it. + */ +export function withoutExtensionOwnershipTest(sql: string): string { + const withoutSchemas = sql.replace( + /\s+AND\s+[\w.]+ NOT IN \(SELECT n\.nspname FROM pg_namespace n JOIN pg_depend[^)]*\)/g, + "", + ); + return withoutSchemas.replace( + /\s+AND\s+\([\w.]+,\s*[\w.]+\) NOT IN \(SELECT n\.nspname,\s*c\.relname FROM pg_class c JOIN pg_namespace n[^)]*\)/g, + "", + ); +} + /** * The column inventory, one row per TABLE with its columns aggregated (B52). * @@ -189,6 +302,34 @@ function equalsClause(column: string, value: string | undefined, field: string, * bounds the prompt independently (`MAX_COLUMNS_PER_TABLE`, * `AGENT_CONTEXT_PACK_MAX_CHARS`), so a wide table does not spend the context * window on itself. + * + * This change closes B76: the aggregation that fixed the wide-catalog refusal + * then admitted the image's own objects. The WHERE clause below now excludes + * them three ways — the full engine-builtin schema list plus every schema an + * extension created (both copied from the provider's object browser), AND every + * RELATION an extension created (`pg_depend` with `classid = 'pg_class'::regclass`, + * new on the agent path). The relation test is the load-bearing half on AlloyDB + * Omni, whose 67 extension views mostly sit in `public` itself: no schema + * filter can reach them, an ownership test can, and a user's own views are + * never extension-owned so they survive. + * + * Every PostgreSQL catalog read carries the relation test, not only this one: + * `inspect_schema` serves `indexes`, `statistics` and `relations` straight to + * the model without passing them through `buildPostgresTables`, so a read that + * filtered by schema alone would show a PostGIS-owned `spatial_ref_sys` in the + * index and statistics inventories while hiding it from the column inventory. + * The exclusion is applied where each statement identifies its own table, and + * the four reads agree on the object set. + * + * Measured live on 2026-09-07 against the three `compat` images with two user + * tables seeded: the column read answers 46 → 2 object rows on TimescaleDB, + * 67 → 2 on Cloudberry and 70 → 2 on AlloyDB Omni. The sharp case is proven on + * AlloyDB, not assumed: the schema half alone leaves 51 objects (the 2 user + * tables plus the 49 extension views installed into `public`), `pg_depend` + * reports 68 extension-owned relations there, and the relation ownership test + * is what removes them. The committed tests pin the SQL SHAPE (the composed + * text carries each filter); the row counts above are the live BEHAVIOUR, and + * the two are asserted at different layers for that reason. */ function composePostgresCatalog(selector: AgentCatalogSelector): string { return ( @@ -196,7 +337,8 @@ function composePostgresCatalog(selector: AgentCatalogSelector): string { "'name', column_name, 'type', data_type, 'nullable', is_nullable) " + "ORDER BY ordinal_position) AS columns " + "FROM information_schema.columns " + - "WHERE table_schema NOT IN ('pg_catalog', 'information_schema')" + + `WHERE ${postgresSchemaExclusion("table_schema")}` + + ` AND ${postgresRelationExclusion("table_schema", "table_name")}` + equalsClause("table_schema", selector.schema, "schema", "postgres") + equalsClause("table_name", selector.table, "table", "postgres") + " GROUP BY table_schema, table_name ORDER BY table_schema, table_name" @@ -250,7 +392,9 @@ function composePostgresRelations(selector: AgentCatalogSelector): string { "JOIN LATERAL unnest(c.conkey, c.confkey) WITH ORDINALITY AS k(attnum, fattnum, ord) ON true " + "JOIN pg_attribute att ON att.attrelid = c.conrelid AND att.attnum = k.attnum " + "JOIN pg_attribute fatt ON fatt.attrelid = c.confrelid AND fatt.attnum = k.fattnum " + - "WHERE c.contype = 'f' AND rn.nspname NOT IN ('pg_catalog', 'information_schema')" + + "WHERE c.contype = 'f' AND " + + postgresSchemaExclusion("rn.nspname") + + ` AND ${postgresRelationExclusion("rn.nspname", "rel.relname")}` + equalsClause("rn.nspname", selector.schema, "schema", "postgres") + equalsClause("rel.relname", selector.table, "table", "postgres") + " ORDER BY rn.nspname, rel.relname, k.ord" @@ -302,7 +446,8 @@ function composePostgresIndexes(selector: AgentCatalogSelector): string { "JOIN pg_namespace n ON n.oid = t.relnamespace " + "JOIN LATERAL unnest(ix.indkey) WITH ORDINALITY AS k(attnum, ord) ON true " + "LEFT JOIN pg_attribute att ON att.attrelid = t.oid AND att.attnum = k.attnum " + - "WHERE n.nspname NOT IN ('pg_catalog', 'information_schema', 'pg_toast')" + + `WHERE ${postgresSchemaExclusion("n.nspname")}` + + ` AND ${postgresRelationExclusion("n.nspname", "t.relname")}` + equalsClause("n.nspname", selector.schema, "schema", "postgres") + equalsClause("t.relname", selector.table, "table", "postgres") + " ORDER BY n.nspname, t.relname, i.relname, k.ord" @@ -355,7 +500,9 @@ function composePostgresStatistics(selector: AgentCatalogSelector): string { "JOIN pg_namespace n ON n.oid = c.relnamespace " + "LEFT JOIN pg_stats s ON s.schemaname = n.nspname AND s.tablename = c.relname " + "WHERE c.relkind IN ('r', 'p') " + - "AND n.nspname NOT IN ('pg_catalog', 'information_schema', 'pg_toast')" + + "AND " + + postgresSchemaExclusion("n.nspname") + + ` AND ${postgresRelationExclusion("n.nspname", "c.relname")}` + equalsClause("n.nspname", selector.schema, "schema", "postgres") + equalsClause("c.relname", selector.table, "table", "postgres") + " ORDER BY n.nspname, c.relname, s.attname" diff --git a/src/lib/agent/tools.ts b/src/lib/agent/tools.ts index 3c293f0c..388d9870 100644 --- a/src/lib/agent/tools.ts +++ b/src/lib/agent/tools.ts @@ -84,6 +84,7 @@ import { AgentComposedSqlError, composeCatalogRead, composeEstimatingExplain, + withoutExtensionOwnershipTest, } from "./composed-sql"; import type { AgentDeadlineDenyCode, AgentRunDeadline } from "./deadline"; import { @@ -2271,6 +2272,18 @@ export async function readStatementForGrounding( }); } +/** + * Whether a database error names the two catalogs the extension ownership tests + * join against. A postgres-typed connection can reach an engine the driver + * serves but nobody here has run, and one without those catalogs raises on + * them; the catalog read then retries without the ownership tests — the same + * decision the provider makes in `isMissingExtensionCatalogError`. + */ +function isMissingExtensionCatalogError(message: string): boolean { + const lowered = message.toLowerCase(); + return lowered.includes("pg_depend") || lowered.includes("pg_extension"); +} + async function readCatalog( context: AgentToolContext, selectorSchema: z.ZodType<{ kind?: AgentCatalogKind; schema?: string; table?: string }>, @@ -2286,7 +2299,7 @@ async function readCatalog( } catch (error) { return composedSqlOutcome(error); } - const outcome = await executeAgentOperation(context, { + const request: AgentOperationRequest = { operationId: "sql.query.read", sql, grounding, @@ -2317,7 +2330,24 @@ async function readCatalog( // case-sensitively, so declaring a raw `MAIN` would compose fine and then be // denied against a `["main"]` allowlist. ...(selector.schema === undefined ? {} : { target: { schema: normalizeDeclaredSchema(context, selector.schema) } }), - }); + }; + let outcome = await executeAgentOperation(context, request); + + // A postgres-typed connection can reach an engine the driver serves but nobody + // here has run, one without the two catalogs the ownership tests join against + // (it raises on pg_depend/pg_extension). The provider drops the tests on that + // error and retries with the fixed schema list alone; mirror it here. + if ( + context.connection.type === "postgres" && + outcome.kind === "refused" && + outcome.refusal.class === "database-error" && + isMissingExtensionCatalogError(outcome.refusal.message) + ) { + const fallback = withoutExtensionOwnershipTest(sql); + if (fallback !== sql) { + outcome = await executeAgentOperation(context, { ...request, sql: fallback }); + } + } /* A catalog read that matched NO OBJECT is a refusal, not a result. diff --git a/tests/unit/lib/agent/composed-sql.test.ts b/tests/unit/lib/agent/composed-sql.test.ts index 36f1f1f0..4880247a 100644 --- a/tests/unit/lib/agent/composed-sql.test.ts +++ b/tests/unit/lib/agent/composed-sql.test.ts @@ -6,9 +6,10 @@ import { composeEstimatingExplain, composeStatisticsAvailabilityProbe, MAX_CATALOG_SELECTOR_LENGTH, + withoutExtensionOwnershipTest, } from "@/lib/agent/composed-sql"; import { agentReadSqlInput, inspectAgentStatement } from "@/lib/db/operations/statement-guard"; -import { mkdtempSync, rmSync } from "node:fs"; +import { mkdtempSync, readFileSync, rmSync } from "node:fs"; import { tmpdir } from "node:os"; import { join } from "node:path"; import { quoteLiteral } from "@/lib/sql/values"; @@ -82,6 +83,82 @@ describe("composeCatalogRead — PostgreSQL", () => { // object, never as top-level select items. expect(sql).not.toContain("column_name, data_type, is_nullable"); }); + + test("excludes the engine's own schemas the provider's browser already excludes (B76)", () => { + const sql = composeCatalogRead("postgres", {}); + + // The full engine-builtin list, not just pg_catalog/information_schema: a + // TimescaleDB chunk schema and Cloudberry's gp_toolkit are grounding noise too. + for (const schema of [ + "pg_toast", + "mz_catalog", + "crdb_internal", + "pg_extension", + "_timescaledb_internal", + "gp_toolkit", + "pg_ext_aux", + ]) { + expect(sql, schema).toContain(`'${schema}'`); + } + }); + + test("excludes schemas an extension created, by ownership and not by name (B76)", () => { + const sql = composeCatalogRead("postgres", {}); + + expect(sql).toContain("pg_depend"); + expect(sql).toContain("'pg_namespace'::regclass"); + expect(sql).toContain("deptype = 'e'"); + }); + + test("excludes relations an extension created, so AlloyDB's public extension views drop out (B76)", () => { + const sql = composeCatalogRead("postgres", {}); + + // The sharp case: no schema filter reaches an object installed into `public`. + expect(sql).toContain("(table_schema, table_name) NOT IN"); + expect(sql).toContain("'pg_class'::regclass"); + expect(sql).toContain("deptype = 'e'"); + }); + + test("does not use a blanket table_type filter, so a user's own views survive (B76)", () => { + const sql = composeCatalogRead("postgres", {}); + + // A table_type filter would also drop the user's views. The ownership test + // drops only what an extension created. + expect(sql).not.toContain("table_type"); + }); + + test("every PostgreSQL kind carries the relation ownership test (B76)", () => { + for (const kind of ["columns", "relations", "indexes", "statistics"] as const) { + const sql = composeCatalogRead("postgres", { kind }); + + // The relation test is the only thing in any composed statement that names + // `pg_class` ownership, so its presence is what pins the test to the kind. + expect(sql, kind).toContain("'pg_class'::regclass"); + expect(sql, kind).toContain("deptype = 'e'"); + expect(guardAccepts(sql), kind).toBe(true); + } + }); + + test("the extension ownership tests strip down to the fixed schema list (fallback for unprobed engines)", () => { + for (const kind of ["columns", "relations", "indexes", "statistics"] as const) { + const sql = composeCatalogRead("postgres", { kind }); + const stripped = withoutExtensionOwnershipTest(sql); + + // The ownership joins are gone, the fixed list remains, and the stripped + // statement is still one the bounded-read guard admits. `pg_extension` stays + // as a schema name in the fixed list (CockroachDB); the JOIN against it is + // what must disappear. + expect(stripped, kind).not.toContain("pg_depend"); + expect(stripped, kind).not.toContain("regclass"); + expect(stripped, kind).not.toContain("JOIN pg_extension"); + expect(stripped, kind).toContain("'pg_catalog'"); + expect(guardAccepts(stripped), kind).toBe(true); + } + }); + + test("stripping a statement without the ownership tests is a no-op", () => { + expect(withoutExtensionOwnershipTest("SELECT 1")).toBe("SELECT 1"); + }); }); describe("composeCatalogRead — SQLite", () => { @@ -1015,3 +1092,38 @@ describe("the composers are reachable, which is the defect that put this file he expect(() => composeEstimatingExplain("duckdb", "SELECT 1")).not.toThrow(AgentComposedSqlError); }); }); + +describe("composeCatalogRead — the agent's exclusion set cannot drift from the provider's (B76)", () => { + const quotedNames = (block: string | undefined): string[] => + block === undefined ? [] : [...block.matchAll(/"([^"]+)"/g)].map((match) => match[1]); + + const readSource = (relativePath: string): string => readFileSync(join(process.cwd(), relativePath), "utf8"); + + test("POSTGRES_SYSTEM_SCHEMAS is the same list the provider ships", () => { + const providerSchemas = quotedNames( + /const SYSTEM_SCHEMAS = \[([\s\S]*?)\] as const;/.exec(readSource("src/lib/db/providers/sql/postgres.ts"))?.[1], + ); + const agentSchemas = quotedNames( + /const POSTGRES_SYSTEM_SCHEMAS = \[([\s\S]*?)\] as const;/.exec(readSource("src/lib/agent/composed-sql.ts"))?.[1], + ); + + // Non-vacuity first: a regex that stops matching silently turns both lists + // empty and the assertion below into a tautology. + expect(providerSchemas.length).toBeGreaterThan(0); + expect(agentSchemas).toEqual(providerSchemas); + }); + + test("the extension-owned schema query is the provider's, verbatim", () => { + const providerQuery = /const EXTENSION_OWNED_SCHEMAS_SQL =([\s\S]*?);/.exec( + readSource("src/lib/db/providers/sql/postgres.ts"), + )?.[1]; + const agentQuery = /const POSTGRES_EXTENSION_OWNED_SCHEMAS_SQL =([\s\S]*?);/.exec( + readSource("src/lib/agent/composed-sql.ts"), + )?.[1]; + + const normalize = (query: string | undefined): string => (query ?? "").replace(/\s+/g, " ").trim(); + + expect(normalize(providerQuery).length).toBeGreaterThan(0); + expect(normalize(agentQuery)).toBe(normalize(providerQuery)); + }); +}); diff --git a/tests/unit/lib/agent/context-snapshot.test.ts b/tests/unit/lib/agent/context-snapshot.test.ts index e740e824..d9a06af4 100644 --- a/tests/unit/lib/agent/context-snapshot.test.ts +++ b/tests/unit/lib/agent/context-snapshot.test.ts @@ -438,6 +438,104 @@ describe("captureContextSnapshot — wide PostgreSQL catalogs (B52)", () => { }); }); +describe("captureContextSnapshot — the capture excludes each image's own extension objects (B76)", () => { + const SHAPES = [ + { + name: "TimescaleDB", + // The fixed schema list is what removes this row. + fragment: "'_timescaledb_internal'", + extensionRows: [ + { + table_schema: "_timescaledb_internal", + table_name: "_hyper_1_1_chunk", + columns: [{ name: "time", type: "timestamptz", nullable: "NO" }], + }, + ], + excludedBy: (sql: string) => sql.includes("'_timescaledb_internal'"), + }, + { + name: "Cloudberry", + fragment: "'gp_toolkit'", + extensionRows: [ + { + table_schema: "gp_toolkit", + table_name: "gp_stats_missing", + columns: [{ name: "relname", type: "name", nullable: "YES" }], + }, + ], + excludedBy: (sql: string) => sql.includes("'gp_toolkit'"), + }, + { + name: "AlloyDB Omni", + // public is not a schema to exclude; only the relation ownership test can + // reach an object installed there. + fragment: "'pg_class'::regclass", + extensionRows: [ + { + table_schema: "public", + table_name: "google_db_advisor_reports", + columns: [{ name: "id", type: "integer", nullable: "NO" }], + }, + ], + excludedBy: (sql: string) => sql.includes("'pg_class'::regclass"), + }, + ]; + + test("the column read carries the full engine-schema list and both ownership tests", async () => { + const h = harness("postgres"); + + await captureContextSnapshot(h.context); + + const columnRead = h.statements().find((sql) => sql.includes("information_schema.columns")); + expect(columnRead).toBeDefined(); + + // The full engine-builtin list, copied from the provider's object browser. + for (const schema of [ + "pg_toast", + "_timescaledb_internal", + "gp_toolkit", + "pg_ext_aux", + "mz_catalog", + "crdb_internal", + "pg_extension", + ]) { + expect(columnRead, schema).toContain(`'${schema}'`); + } + // Both ownership tests: the schema one (google_ml/ai) and the relation one + // (AlloyDB's public extension views). + expect(columnRead).toContain("'pg_namespace'::regclass"); + expect(columnRead).toContain("'pg_class'::regclass"); + expect(columnRead).toContain("deptype = 'e'"); + }); + + for (const shape of SHAPES) { + test(`a ${shape.name}-shaped database reaches the fold with its internal objects excluded`, async () => { + const h = harness("postgres", async (sql) => { + if (sql.includes("information_schema.columns")) { + // The engine applies the composed filter before answering. The harness + // mirrors only this shape's own exclusion, so swapping the shapes swaps + // the row that is removed — each case is distinct, not a shared string. + return result(shape.excludedBy(sql) ? PG_COLUMNS : [...PG_COLUMNS, ...shape.extensionRows]); + } + return result([]); + }); + + const capture = await captureContextSnapshot(h.context); + + expect(capture.kind).toBe("captured"); + if (capture.kind !== "captured") throw new Error("unreachable"); + + // The shape-specific fragment is present in the composed column read, and + // the fold names only the user's tables — the extension row never survives. + const columnRead = h.statements().find((sql) => sql.includes("information_schema.columns")); + expect(columnRead, shape.name).toBeDefined(); + expect(columnRead, shape.name).toContain(shape.fragment); + + expect(capture.snapshot.tables.map((table) => table.name).sort()).toEqual(["public.customers", "public.orders"]); + }); + } +}); + describe("captureContextSnapshot — SQLite", () => { test("takes two reads, because the table DDL carries the relations as well", async () => { const h = harness("sqlite"); diff --git a/tests/unit/lib/agent/tools.test.ts b/tests/unit/lib/agent/tools.test.ts index d67f5709..a93aea21 100644 --- a/tests/unit/lib/agent/tools.test.ts +++ b/tests/unit/lib/agent/tools.test.ts @@ -2825,6 +2825,38 @@ describe("the grounding seam — the server's own read, outside agent mode", () }); }); +describe("readCatalog — the extension-ownership fallback retry", () => { + test("a database error naming pg_depend retries once without the ownership tests", async () => { + let attempts = 0; + const h = harness({}, async () => { + attempts += 1; + if (attempts === 1) throw new QueryError('relation "pg_depend" does not exist', "postgres"); + return queryResult(); + }); + + const outcome = await readCatalogForGrounding(h.context, {}); + + expect(outcome.kind).toBe("completed"); + expect(h.queryReadOnly).toHaveBeenCalledTimes(2); + const first = h.queryReadOnly.mock.calls[0][0] as string; + const second = h.queryReadOnly.mock.calls[1][0] as string; + expect(first).toContain("pg_depend"); + expect(second).not.toContain("pg_depend"); + expect(second).toContain("information_schema.columns"); + }); + + test("a database error that does not name those catalogs is not retried", async () => { + const h = harness({}, async () => { + throw new QueryError('column "ordr_id" does not exist', "postgres"); + }); + + const outcome = await readCatalogForGrounding(h.context, {}); + + expect(outcome.kind).toBe("refused"); + expect(h.queryReadOnly).toHaveBeenCalledTimes(1); + }); +}); + describe("inspectPlanTool — the estimating variant only", () => { test("composes the estimating EXPLAIN for the connection's dialect", async () => { const h = harness();