From e10339f85ae79174f01264b56050f80aab6d5c02 Mon Sep 17 00:00:00 2001 From: sloemodzn Date: Sat, 19 Sep 2026 07:27:45 +0530 Subject: [PATCH 1/2] fix(agent): read the sqlite kind vocabulary from the provider, not source text The guard that catches a SQLite object kind declared but left unmapped read `SQLITE_OBJECT_KINDS` out of `sqlite.ts` with a `{ id: "..."` scrape that matches a single-line entry only. A fifth kind written across two lines was therefore absent from the guard's population AND from `COMPOSED_KIND_WORDS`, both sides shrank together, and the guard passed over a kind nothing maps. Measured against the same declaration, using the same pattern: today, unchanged scrape sees 4 -> guard passes a FIFTH kind wrapped scrape sees 4 -> guard passes the same FIFTH kind on one line scrape sees 5 -> guard fails on it The guard now asks the constructed provider for `getCapabilities().objectKinds`, the durable fix the array's own comment named. Capabilities are type-driven and read off a provider that is never connected, so no socket is opened. An entry that wraps can no longer hide a kind, which is what the new test pins, alongside the unmapped-kind failure itself. The one-line-entries constraint and its rationale leave the comment in `sqlite.ts`, since nothing reads that array as text any more. --- src/lib/db/providers/sql/sqlite.ts | 23 +++----- tests/unit/lib/agent/context-snapshot.test.ts | 54 ++++++++++++++++--- 2 files changed, 55 insertions(+), 22 deletions(-) diff --git a/src/lib/db/providers/sql/sqlite.ts b/src/lib/db/providers/sql/sqlite.ts index cf081d299..d8edefe73 100644 --- a/src/lib/db/providers/sql/sqlite.ts +++ b/src/lib/db/providers/sql/sqlite.ts @@ -335,22 +335,13 @@ const OBJECT_FOREIGN_KEYS_SQL = ` * on this engine that declares nothing. `sql` is a Monaco language id the installed bundle * really registers, which `plsql`, `tsql` and `cql` are not. * - * The name is short so each entry below stays ONE LINE, which is not cosmetic: - * `tests/unit/lib/agent/context-snapshot.test.ts` reads the declared ids out of this array - * with a `{ id: "..."` scrape that matches a SINGLE-LINE entry only. - * - * What that scrape does and does not notice, both measured on 2026-09-13 rather than argued. - * Writing `hasSource: true, sourceLanguage: "sql"` inline pushes the `table` and `trigger` - * entries past 120 columns and the formatter explodes them; the guard then holds two declared - * ids against the agent side's four and FAILS LOUDLY (1 fail, "Expected - 0 / Received + 2", - * `table` and `trigger` unmatched). So an exploded EXISTING entry is a red test and not a - * silent loss. The silent case is the other one, and it is the reason these entries are kept - * scrapable: a FIFTH kind written as a multi-line entry is missing from the guard's population - * AND from the agent side's map, both sides shrink together, and the guard passes over a kind - * nothing maps (measured: multi-line fifth kind 1 pass 0 fail, the same kind on one line - * 1 fail). The durable fix is not here, because that file belongs to the agent side: the guard - * should read the declaration through `createDatabaseProvider("sqlite").getCapabilities()` - * instead of scraping this text. Filed in the backlog under #789. + * The layout of these entries is free: nothing reads them as text. The vocabulary is pinned to + * the agent side's `COMPOSED_KIND_WORDS` by `tests/unit/lib/agent/context-snapshot.test.ts`, + * which asks the constructed provider for `getCapabilities().objectKinds` rather than scraping + * this array (#981). An entry that wraps therefore cannot hide its kind from that guard, which + * is the hole the earlier `{ id: "..."` scrape left: a fifth kind written across two lines was + * missing from the guard's population AND from the agent side's map, so both sides shrank + * together and the guard passed over a kind nothing maps. */ const SOURCE_SQL: Pick = { hasSource: true, sourceLanguage: "sql" }; diff --git a/tests/unit/lib/agent/context-snapshot.test.ts b/tests/unit/lib/agent/context-snapshot.test.ts index 85b4dd3b6..19e129d7d 100644 --- a/tests/unit/lib/agent/context-snapshot.test.ts +++ b/tests/unit/lib/agent/context-snapshot.test.ts @@ -19,6 +19,7 @@ import type { AgentToolContext } from "@/lib/agent/tools"; import type { AgentContextSnapshot, AgentRunEvent } from "@/lib/agent/types"; import { UNTRUSTED_CONTENT_BEGIN, UNTRUSTED_CONTENT_END } from "@/lib/agent/untrusted-content"; import { ConnectionError, ExecutionProfileError, QueryError } from "@/lib/db/errors"; +import { createDatabaseProvider } from "@/lib/db/factory"; import { measureResultBytes } from "@/lib/db/providers/sql/read-only-budget"; import { ExecutionArtifactStore } from "@/lib/db/operations/artifacts"; import { ExecutionBudgetTracker } from "@/lib/db/operations/budgets"; @@ -2278,6 +2279,35 @@ describe("the composed kind vocabulary cannot drift from the provider's declarat return Object.fromEntries([...(block ?? "").matchAll(/(\w+): "(\w+)"/g)].map((match) => [match[1], match[2]])); }; + /** + * The provider's own sqlite kind ids (#981). Asking the constructed provider rather than + * reading `sqlite.ts` as text is what makes the guard independent of how the declaration + * is laid out: the scrape it replaced matched a single-line entry only. + * + * Capabilities are type-driven, so this needs no socket - the same reason the + * `provider-meta` route can read them off a provider it never connects. + */ + const declaredSqliteKindIds = async (): Promise => { + const provider = await createDatabaseProvider({ + id: "kind-vocabulary-guard", + name: "kind-vocabulary-guard", + type: "sqlite", + // The provider validates its config in the constructor and wants a file path, even + // though capabilities are read without ever connecting. `:memory:` is the documented + // way to satisfy that without naming a file. + database: ":memory:", + createdAt: new Date(0), + } satisfies DatabaseConnection); + return (provider.getCapabilities().objectKinds ?? []).map((kind) => kind.id); + }; + + /** + * The guard's comparison, in one place so the failure it exists to produce can be + * exercised on its own: every declared kind id must be its own word in the agent side's map. + */ + const unmappedKindIds = (declaredIds: readonly string[], composed: Record): string[] => + declaredIds.filter((id) => composed[id] !== id); + test("PostgreSQL: every relkind is mapped exactly as the provider's own CASE maps it", () => { // `COUNTS_RELATION_ARM` is the CASE `postgres.ts` takes its own folder counts and // listings from, so a relation this path calls a view is one its object browser @@ -2295,18 +2325,30 @@ describe("the composed kind vocabulary cannot drift from the provider's declarat expect(composedMap("postgres")).toEqual(providerMap); }); - test("SQLite: the four words sqlite_schema types objects with are the four ids declared", () => { - const declaredIds = [ - ...( - /const SQLITE_OBJECT_KINDS[\s\S]*?\n\];/.exec(readSource("src/lib/db/providers/sql/sqlite.ts"))?.[0] ?? "" - ).matchAll(/\{ id: "(\w+)"/g), - ].map((match) => match[1]); + test("SQLite: the four words sqlite_schema types objects with are the four ids declared", async () => { + // Read through the provider rather than this file's text (#981). The scrape this + // replaced matched a single-line `{ id: "..."` entry only, so a kind declared across + // two lines left BOTH sides one word shorter and the guard passed over a kind nothing + // maps. The provider answers the same question without caring how the array is laid out. + const declaredIds = await declaredSqliteKindIds(); expect(declaredIds.length).toBeGreaterThan(0); // The identity map, which is the claim: `sqlite_schema.type` and the declared kind // ids are the same vocabulary, so neither side may gain a word alone. expect(composedMap("sqlite")).toEqual(Object.fromEntries(declaredIds.map((id) => [id, id]))); }); + + test("a kind declared but left unmapped is caught, and the declaration's formatting cannot hide it", () => { + const composed = composedMap("sqlite"); + + // The fifth kind that nothing maps. This is the case the source scrape could not see + // once its entry wrapped: absent from the guard's population AND from the agent side's + // map, so both sides shrank together and the guard passed (#981). + expect(unmappedKindIds([...Object.keys(composed), "fifth_kind"], composed)).toEqual(["fifth_kind"]); + // And the declared vocabulary it does carry stays clean, so the check above is not + // passing because it reports everything. + expect(unmappedKindIds(Object.keys(composed), composed)).toEqual([]); + }); }); describe("an inventory that knows what its objects ARE", () => { From ec5fd2f924b638703cfa74038597719f22bff073 Mon Sep 17 00:00:00 2001 From: sloemodzn Date: Sat, 19 Sep 2026 19:25:58 +0530 Subject: [PATCH 2/2] test(agent): control plus mutant over the real declaration for the kind guard Review feedback: the previous test's population was built from composedMap plus a literal, so it never reached the declaration or the provider and only asserted the filter. Replaced with the control plus mutant over declaredSqliteKindIds() and the real composed map, so the comparison itself is what stops holding when a fifth kind appears. unmappedKindIds is dropped with it, and the describe docblock no longer claims a source-level read for the sqlite arm. --- tests/unit/lib/agent/context-snapshot.test.ts | 26 +++++++------------ 1 file changed, 9 insertions(+), 17 deletions(-) diff --git a/tests/unit/lib/agent/context-snapshot.test.ts b/tests/unit/lib/agent/context-snapshot.test.ts index 19e129d7d..cbec72b94 100644 --- a/tests/unit/lib/agent/context-snapshot.test.ts +++ b/tests/unit/lib/agent/context-snapshot.test.ts @@ -2266,8 +2266,9 @@ describe("captureContextSnapshot — the kind, composed on the catalog path", () * id, or the run and the tree disagree about what a thing is. The right-hand side of * `COMPOSED_KIND_WORDS` is therefore a copy of each provider's own mapping, and a copy * that nothing checks is a copy that drifts - which is why `POSTGRES_SYSTEM_SCHEMAS` is - * pinned the same way in `composed-sql.test.ts`. Source-level, because the agent side - * must not import a provider module. + * pinned the same way in `composed-sql.test.ts`. The postgres arm stays source-level, + * because the agent side must not import a provider module; the sqlite arm asks the + * constructed provider instead (#981). */ describe("the composed kind vocabulary cannot drift from the provider's declaration", () => { const readSource = (relativePath: string): string => readFileSync(join(process.cwd(), relativePath), "utf8"); @@ -2301,13 +2302,6 @@ describe("the composed kind vocabulary cannot drift from the provider's declarat return (provider.getCapabilities().objectKinds ?? []).map((kind) => kind.id); }; - /** - * The guard's comparison, in one place so the failure it exists to produce can be - * exercised on its own: every declared kind id must be its own word in the agent side's map. - */ - const unmappedKindIds = (declaredIds: readonly string[], composed: Record): string[] => - declaredIds.filter((id) => composed[id] !== id); - test("PostgreSQL: every relkind is mapped exactly as the provider's own CASE maps it", () => { // `COUNTS_RELATION_ARM` is the CASE `postgres.ts` takes its own folder counts and // listings from, so a relation this path calls a view is one its object browser @@ -2338,16 +2332,14 @@ describe("the composed kind vocabulary cannot drift from the provider's declarat expect(composedMap("sqlite")).toEqual(Object.fromEntries(declaredIds.map((id) => [id, id]))); }); - test("a kind declared but left unmapped is caught, and the declaration's formatting cannot hide it", () => { + test("a fifth declared kind that nothing maps fails the guard, however its entry is laid out", async () => { const composed = composedMap("sqlite"); + const declaredIds = await declaredSqliteKindIds(); - // The fifth kind that nothing maps. This is the case the source scrape could not see - // once its entry wrapped: absent from the guard's population AND from the agent side's - // map, so both sides shrank together and the guard passed (#981). - expect(unmappedKindIds([...Object.keys(composed), "fifth_kind"], composed)).toEqual(["fifth_kind"]); - // And the declared vocabulary it does carry stays clean, so the check above is not - // passing because it reports everything. - expect(unmappedKindIds(Object.keys(composed), composed)).toEqual([]); + // Control: the real declaration agrees, so the mutant below is the only difference. + expect(composed).toEqual(Object.fromEntries(declaredIds.map((id) => [id, id]))); + // Mutant: one more declared kind, and the SAME comparison must stop holding. + expect(composed).not.toEqual(Object.fromEntries([...declaredIds, "fifth_kind"].map((id) => [id, id]))); }); });