fix(agent): exclude engine and extension objects from grounding capture - #625
fix(agent): exclude engine and extension objects from grounding capture#625koraysrn wants to merge 1 commit into
Conversation
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.
Codecov Report✅ All modified and coverable lines are covered by tests. 📢 Thoughts on this report? Let us know! |
cevheri
added
documentation
cevheri
left a comment
cevheri
left a comment
There was a problem hiding this comment.
Reviewed the diff and ran the two touched test files locally: 189 pass, 0 fail.
Verified correct: the NOT IN composition has no AND/OR precedence hazard in any of the four statements, the copied POSTGRES_SYSTEM_SCHEMAS list is byte-identical to the provider's, the drift guard is non-vacuous, the B2-B75 - 21 count matches the 21 remaining ### B entries, and no stale B76 references are left.
Five comments below. The two in composed-sql.ts are behaviour: a missing fallback that breaks the agent path on Materialize, and an exclusion applied to one of the three catalog reads. The other three are a doc claim, a markdown break and a nominal test.
| * 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 = |
There was a problem hiding this comment.
The provider does not read pg_depend / pg_extension unguarded. It carries a fallback: isMissingExtensionCatalogError -> withoutExtensionOwnershipTest (src/lib/db/providers/sql/postgres.ts:458 and :484), because Materialize raises on those catalogs.
The agent path has no equivalent. composeCatalogRead returns one composed statement and there is no rewrite chain around it, so on a connection typed postgres that is actually Materialize (and postgres is in AGENT_EXECUTION_ENGINES) all four catalog reads fail: the context capture goes silently unavailable and inspect_schema returns a DB failure.
The live verification covered TimescaleDB, Cloudberry and AlloyDB Omni, all of which do have pg_depend, so it could not see this. Either mirror the provider's fallback or state the Materialize exclusion explicitly.
| "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})` + |
There was a problem hiding this comment.
The relation-ownership test is applied on the columns read only, which leaves the exclusion inconsistent across the reads a run makes:
kind=indexesandkind=statisticsfilter by schema alone and go to the model throughreadCatalog, not throughbuildPostgresTables- the object browser has no
pg_classownership test at all
Concretely, with PostGIS installed, public.spatial_ref_sys stays visible in the browser and in the index and statistics reads, but disappears from the column inventory. Same object, three different answers in one run. Worth either extending the test to the other kinds or recording why columns is the only place it belongs.
| 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 |
There was a problem hiding this comment.
"all copied from this provider's object browser rather than invented on the agent path" is not accurate for the third way. The browser has no pg_class extension-ownership test; that one is new on the agent path. As written the sentence also hides the divergence: the relation test covers the columns read and not indexes / statistics or the browser.
| 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 |
There was a problem hiding this comment.
The B76 removal took the blank line before this paragraph with it, so "Settled as limits rather than as work." now renders as a continuation of the B65 bullet instead of a new paragraph. One blank line above this line fixes it.
| }); | ||
|
|
||
| describe("captureContextSnapshot — the capture excludes each image's own extension objects (B76)", () => { | ||
| const SHAPES = [ |
There was a problem hiding this comment.
These three "per shape" cases pass no fixture rows and assert substrings of the same composed string, so nothing shape-specific is actually exercised. They pass identically if the shapes are swapped. If the intent is a test per shape, each needs rows that only that shape's exclusion removes.
2 participants
Summary
The aggregated grounding capture that fixed the wide-catalog row-cap refusal
(B52) then admitted the image's own extension objects. This closes B76 by
composing the PostgreSQL column read with the same exclusions the provider's
object browser already applies, plus a relation-level ownership test.
What changed
src/lib/agent/composed-sql.tsPOSTGRES_SYSTEM_SCHEMAS(copied from the provider'sSYSTEM_SCHEMAS),POSTGRES_EXTENSION_OWNED_SCHEMAS_SQLandPOSTGRES_EXTENSION_OWNED_RELATIONS_SQL.composePostgresCatalognow filters by the full engine-builtin schema list,every schema an extension created (
pg_dependonpg_namespace,deptype = 'e'), and every relation an extension created (pg_dependonpg_class,deptype = 'e').column read stays the single source of object identity, so the others need
no relation test.
tests/unit/lib/agent/composed-sql.test.ts— pins the SQL shape, guardacceptance, and that the agent's schema list cannot drift from the provider's.
tests/unit/lib/agent/context-snapshot.test.ts— one test per shape(TimescaleDB, Cloudberry, AlloyDB Omni).
docs/BACKLOG.md— B76 removed (B2–B75 · 21).docs/AGENT.md— B76 deferral record removed.docs/providers/postgres.md— agent grounding description now documents thethree-layer exclusion.
Verification
Measured live on 2026-09-07 against the three
compatimages with two usertables seeded:
AlloyDB is the sharp case: the schema half alone leaves 51 objects (2 user
tables plus the 49 extension views installed into
public);pg_dependreports68 extension-owned relations, and the relation ownership test is what removes
them. A user's own views are never extension-owned, so they survive.
Checks
composed-sql.test.ts+context-snapshot.test.ts+backlog-structure.test.ts: 282 pass, 0 failbun run typecheck: passbiome format(changed files): passbun run lint: 0 errors