From 983556042ce5227366981c4debe32aea820eddba Mon Sep 17 00:00:00 2001 From: koraysrn Date: Mon, 7 Sep 2026 14:57:59 +0300 Subject: [PATCH 1/3] fix(agent): exclude engine and extension objects from grounding capture The aggregated column read that fixed the wide-catalog row-cap refusal (B52) then admitted the image's own extension objects, so a grounded run on TimescaleDB, Cloudberry or AlloyDB Omni reasoned over an inventory that was mostly internal noise. Compose the PostgreSQL column read with the same exclusions the provider's object browser already applies, plus a relation-level ownership test that reaches AlloyDB's public-installed extension views: the full engine-builtin schema list, every schema an extension created (pg_depend on pg_namespace, deptype = 'e'), and every relation an extension created (pg_depend on pg_class, deptype = 'e'). The relation, index and statistics reads are aligned to the full schema list; the column read stays the single source of object identity, so the others need no relation test (their rows attach to the column inventory). Measured live on 2026-09-07 against the three compat images with two user tables seeded: 46 -> 2 object rows on TimescaleDB, 67 -> 2 on Cloudberry, 70 -> 2 on AlloyDB Omni; on AlloyDB the schema half alone leaves 51 and pg_depend reports 68 extension-owned relations. Closes B76. --- docs/AGENT.md | 7 - docs/BACKLOG.md | 30 +---- docs/providers/postgres.md | 19 ++- src/lib/agent/composed-sql.ts | 121 +++++++++++++++++- tests/unit/lib/agent/composed-sql.test.ts | 80 +++++++++++- tests/unit/lib/agent/context-snapshot.test.ts | 61 +++++++++ 6 files changed, 271 insertions(+), 47 deletions(-) diff --git a/docs/AGENT.md b/docs/AGENT.md index ea1d803e..221af475 100644 --- a/docs/AGENT.md +++ b/docs/AGENT.md @@ -2501,13 +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 it, rather than a queue item nobody was going to pick up. A limitation needs a record; it does not 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..41b93d71 100644 --- a/docs/providers/postgres.md +++ b/docs/providers/postgres.md @@ -1083,12 +1083,19 @@ 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, all copied from this provider's object + browser rather than invented on the agent path: the full engine-builtin schema list (not only + `pg_catalog` / `information_schema`), every schema an extension created (`pg_depend` on + `pg_namespace`, `deptype = 'e'`), and every relation an extension created (`pg_depend` on + `pg_class`, `deptype = 'e'`). 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..8fe8f692 100644 --- a/src/lib/agent/composed-sql.ts +++ b/src/lib/agent/composed-sql.ts @@ -154,6 +154,89 @@ 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 column inventory, one row per TABLE with its columns aggregated (B52). * @@ -189,6 +272,33 @@ 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 two ways, both taken from the provider's object browser rather than + * invented here — the full engine-builtin schema list plus every schema an + * extension created, AND every RELATION an extension created (`pg_depend` with + * `classid = 'pg_class'::regclass`). 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. + * + * Only the COLUMN read carries all three tests. The relation, index and + * statistics reads filter by schema alone, and that is complete rather than a + * gap: `buildPostgresTables` attaches a relation or index row to a table that + * is already in the column inventory, so a row for an extension-owned object + * the column read excluded has nothing to attach to and is dropped by the + * fold. The column read is the single source of object identity. + * + * 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 +306,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 (table_schema, table_name) NOT IN (${POSTGRES_EXTENSION_OWNED_RELATIONS_SQL})` + 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 +361,8 @@ 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") + equalsClause("rn.nspname", selector.schema, "schema", "postgres") + equalsClause("rel.relname", selector.table, "table", "postgres") + " ORDER BY rn.nspname, rel.relname, k.ord" @@ -302,7 +414,7 @@ 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")}` + 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 +467,8 @@ 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") + 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/tests/unit/lib/agent/composed-sql.test.ts b/tests/unit/lib/agent/composed-sql.test.ts index 36f1f1f0..29025454 100644 --- a/tests/unit/lib/agent/composed-sql.test.ts +++ b/tests/unit/lib/agent/composed-sql.test.ts @@ -8,7 +8,7 @@ import { MAX_CATALOG_SELECTOR_LENGTH, } 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 +82,49 @@ 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"); + }); }); describe("composeCatalogRead — SQLite", () => { @@ -1015,3 +1058,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..953f64e6 100644 --- a/tests/unit/lib/agent/context-snapshot.test.ts +++ b/tests/unit/lib/agent/context-snapshot.test.ts @@ -438,6 +438,67 @@ describe("captureContextSnapshot — wide PostgreSQL catalogs (B52)", () => { }); }); +describe("captureContextSnapshot — the capture excludes each image's own extension objects (B76)", () => { + const SHAPES = [ + // TimescaleDB floods through _timescaledb_internal, the chunk schema; the name + // list excludes it before any ownership test runs. + { name: "TimescaleDB", schema: "_timescaledb_internal", ownedRelation: false }, + // Cloudberry floods through gp_toolkit's views, excluded the same way. + { name: "Cloudberry", schema: "gp_toolkit", ownedRelation: false }, + // AlloyDB Omni installs its views into public itself, so the relation + // ownership test is the only thing that reaches them. + { name: "AlloyDB Omni", schema: null, ownedRelation: true }, + ]; + + 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"); + + await captureContextSnapshot(h.context); + + const columnRead = h.statements().find((sql) => sql.includes("information_schema.columns")); + expect(columnRead).toBeDefined(); + + if (shape.schema !== null) { + expect(columnRead, shape.name).toContain(`'${shape.schema}'`); + } + if (shape.ownedRelation) { + // The sharp case: public is not a schema to exclude, so the ownership + // test over pg_class is the only mechanism that can reach these views. + expect(columnRead, shape.name).toContain("(table_schema, table_name) NOT IN"); + expect(columnRead, shape.name).toContain("'pg_class'::regclass"); + } + }); + } +}); + describe("captureContextSnapshot — SQLite", () => { test("takes two reads, because the table DDL carries the relations as well", async () => { const h = harness("sqlite"); From 32b620f276e59fd0fd1a10b517251efd6979e3b5 Mon Sep 17 00:00:00 2001 From: koraysrn Date: Tue, 8 Sep 2026 13:26:32 +0300 Subject: [PATCH 2/3] =?UTF-8?q?fix(agent):=20address=20B76=20review=20?= =?UTF-8?q?=E2=80=94=20Materialize=20fallback=20and=20cross-kind=20consist?= =?UTF-8?q?ency?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Apply the relation ownership test to all four catalog reads, not only columns, so inspect_schema answers the same object set whatever kind it is asked for (PostGIS spatial_ref_sys stays out of indexes and statistics too). Mirror the provider's Materialize fallback: a postgres-typed connection whose engine lacks pg_depend/pg_extension retries without the ownership tests, keeping the fixed schema list (withoutExtensionOwnershipTest). Correct the provider doc (the pg_class ownership test is new on the agent path), restore the blank line before "Settled as limits" in docs/AGENT.md, and give the per-shape tests shape-specific rows rather than shared substrings. --- docs/AGENT.md | 1 + docs/providers/postgres.md | 17 ++-- src/lib/agent/composed-sql.ts | 64 +++++++++++---- src/lib/agent/tools.ts | 33 +++++++- tests/unit/lib/agent/composed-sql.test.ts | 34 ++++++++ tests/unit/lib/agent/context-snapshot.test.ts | 79 ++++++++++++++----- 6 files changed, 182 insertions(+), 46 deletions(-) diff --git a/docs/AGENT.md b/docs/AGENT.md index 221af475..e58845f7 100644 --- a/docs/AGENT.md +++ b/docs/AGENT.md @@ -2501,6 +2501,7 @@ 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. + **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 it, rather than a queue item nobody was going to pick up. A limitation needs a record; it does not diff --git a/docs/providers/postgres.md b/docs/providers/postgres.md index 41b93d71..42f5e30a 100644 --- a/docs/providers/postgres.md +++ b/docs/providers/postgres.md @@ -1085,14 +1085,15 @@ 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 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, all copied from this provider's object - browser rather than invented on the agent path: the full engine-builtin schema list (not only - `pg_catalog` / `information_schema`), every schema an extension created (`pg_depend` on - `pg_namespace`, `deptype = 'e'`), and every relation an extension created (`pg_depend` on - `pg_class`, `deptype = 'e'`). 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. + `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. diff --git a/src/lib/agent/composed-sql.ts b/src/lib/agent/composed-sql.ts index 8fe8f692..ca6f6701 100644 --- a/src/lib/agent/composed-sql.ts +++ b/src/lib/agent/composed-sql.ts @@ -237,6 +237,36 @@ 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`: a postgres-typed connection to an + * engine with no `pg_depend`/`pg_extension` catalogs (Materialize) raises on + * those names, and the read retries without the ownership tests. 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). * @@ -275,20 +305,21 @@ function postgresSchemaExclusion(column: string): string { * * 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 two ways, both taken from the provider's object browser rather than - * invented here — the full engine-builtin schema list plus every schema an - * extension created, AND every RELATION an extension created (`pg_depend` with - * `classid = 'pg_class'::regclass`). 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. - * - * Only the COLUMN read carries all three tests. The relation, index and - * statistics reads filter by schema alone, and that is complete rather than a - * gap: `buildPostgresTables` attaches a relation or index row to a table that - * is already in the column inventory, so a row for an extension-owned object - * the column read excluded has nothing to attach to and is dropped by the - * fold. The column read is the single source of object identity. + * 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, @@ -307,7 +338,7 @@ function composePostgresCatalog(selector: AgentCatalogSelector): string { "ORDER BY ordinal_position) AS columns " + "FROM information_schema.columns " + `WHERE ${postgresSchemaExclusion("table_schema")}` + - ` AND (table_schema, table_name) NOT IN (${POSTGRES_EXTENSION_OWNED_RELATIONS_SQL})` + + ` 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" @@ -363,6 +394,7 @@ function composePostgresRelations(selector: AgentCatalogSelector): string { "JOIN pg_attribute fatt ON fatt.attrelid = c.confrelid AND fatt.attnum = k.fattnum " + "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" @@ -415,6 +447,7 @@ function composePostgresIndexes(selector: AgentCatalogSelector): string { "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 ${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" @@ -469,6 +502,7 @@ function composePostgresStatistics(selector: AgentCatalogSelector): string { "WHERE c.relkind IN ('r', 'p') " + "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..4e4fa78f 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,17 @@ 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 without them + * (Materialize), and 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 +2298,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 +2329,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 without the two catalogs the + // ownership tests join against (Materialize raises on pg_depend/pg_extension). + // The provider drops the tests on that error and retries with the fixed schema + // list alone; mirror that here for the agent path. + 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 29025454..adba9bec 100644 --- a/tests/unit/lib/agent/composed-sql.test.ts +++ b/tests/unit/lib/agent/composed-sql.test.ts @@ -6,6 +6,7 @@ import { composeEstimatingExplain, composeStatisticsAvailabilityProbe, MAX_CATALOG_SELECTOR_LENGTH, + withoutExtensionOwnershipTest, } from "@/lib/agent/composed-sql"; import { agentReadSqlInput, inspectAgentStatement } from "@/lib/db/operations/statement-guard"; import { mkdtempSync, readFileSync, rmSync } from "node:fs"; @@ -125,6 +126,39 @@ describe("composeCatalogRead — PostgreSQL", () => { // 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 (Materialize fallback)", () => { + 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", () => { diff --git a/tests/unit/lib/agent/context-snapshot.test.ts b/tests/unit/lib/agent/context-snapshot.test.ts index 953f64e6..d9a06af4 100644 --- a/tests/unit/lib/agent/context-snapshot.test.ts +++ b/tests/unit/lib/agent/context-snapshot.test.ts @@ -440,14 +440,45 @@ describe("captureContextSnapshot — wide PostgreSQL catalogs (B52)", () => { describe("captureContextSnapshot — the capture excludes each image's own extension objects (B76)", () => { const SHAPES = [ - // TimescaleDB floods through _timescaledb_internal, the chunk schema; the name - // list excludes it before any ownership test runs. - { name: "TimescaleDB", schema: "_timescaledb_internal", ownedRelation: false }, - // Cloudberry floods through gp_toolkit's views, excluded the same way. - { name: "Cloudberry", schema: "gp_toolkit", ownedRelation: false }, - // AlloyDB Omni installs its views into public itself, so the relation - // ownership test is the only thing that reaches them. - { name: "AlloyDB Omni", schema: null, ownedRelation: true }, + { + 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 () => { @@ -479,22 +510,28 @@ describe("captureContextSnapshot — the capture excludes each image's own exten for (const shape of SHAPES) { test(`a ${shape.name}-shaped database reaches the fold with its internal objects excluded`, async () => { - const h = harness("postgres"); + 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); - 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).toBeDefined(); - - if (shape.schema !== null) { - expect(columnRead, shape.name).toContain(`'${shape.schema}'`); - } - if (shape.ownedRelation) { - // The sharp case: public is not a schema to exclude, so the ownership - // test over pg_class is the only mechanism that can reach these views. - expect(columnRead, shape.name).toContain("(table_schema, table_name) NOT IN"); - expect(columnRead, shape.name).toContain("'pg_class'::regclass"); - } + 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"]); }); } }); From e36aed18419498ec4f7b7a60df59b76681c9389e Mon Sep 17 00:00:00 2001 From: koraysrn Date: Tue, 8 Sep 2026 14:38:26 +0300 Subject: [PATCH 3/3] fix(agent): correct fallback wording and test the ownership retry Drop the incorrect Materialize-raises claim: the provider lists Materialize among engines that accept the ownership test, and its fallback exists for PostgreSQL-wire engines nobody has run. Reword the two code comments and the test name to say that instead. Add a readCatalog test that drives a pg_depend database error and asserts the retry sends a second statement without the ownership tests, pinning the trigger condition composed-sql.test.ts cannot reach. --- src/lib/agent/composed-sql.ts | 8 +++--- src/lib/agent/tools.ts | 15 ++++++----- tests/unit/lib/agent/composed-sql.test.ts | 2 +- tests/unit/lib/agent/tools.test.ts | 32 +++++++++++++++++++++++ 4 files changed, 45 insertions(+), 12 deletions(-) diff --git a/src/lib/agent/composed-sql.ts b/src/lib/agent/composed-sql.ts index ca6f6701..75bb2ee8 100644 --- a/src/lib/agent/composed-sql.ts +++ b/src/lib/agent/composed-sql.ts @@ -251,10 +251,10 @@ function postgresRelationExclusion(schemaColumn: string, tableColumn: string): s /** * 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`: a postgres-typed connection to an - * engine with no `pg_depend`/`pg_extension` catalogs (Materialize) raises on - * those names, and the read retries without the ownership tests. Kept as a pure - * string rewrite so the caller decides when to use it. + * 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( diff --git a/src/lib/agent/tools.ts b/src/lib/agent/tools.ts index 4e4fa78f..388d9870 100644 --- a/src/lib/agent/tools.ts +++ b/src/lib/agent/tools.ts @@ -2274,9 +2274,10 @@ 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 without them - * (Materialize), and the catalog read then retries without the ownership tests — - * the same decision the provider makes in `isMissingExtensionCatalogError`. + * 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(); @@ -2332,10 +2333,10 @@ async function readCatalog( }; let outcome = await executeAgentOperation(context, request); - // A postgres-typed connection can reach an engine without the two catalogs the - // ownership tests join against (Materialize raises on pg_depend/pg_extension). - // The provider drops the tests on that error and retries with the fixed schema - // list alone; mirror that here for the agent path. + // 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" && diff --git a/tests/unit/lib/agent/composed-sql.test.ts b/tests/unit/lib/agent/composed-sql.test.ts index adba9bec..4880247a 100644 --- a/tests/unit/lib/agent/composed-sql.test.ts +++ b/tests/unit/lib/agent/composed-sql.test.ts @@ -139,7 +139,7 @@ describe("composeCatalogRead — PostgreSQL", () => { } }); - test("the extension ownership tests strip down to the fixed schema list (Materialize fallback)", () => { + 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); 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();