From 73a3294959f7ddc0e4761976dcc95ca7fce38c9a Mon Sep 17 00:00:00 2001 From: cevheri Date: Sat, 12 Sep 2026 20:39:25 +0300 Subject: [PATCH 01/94] feat(objects): add the object source contract types and derivations (#789) Phase 2 reads an object's definition text. This commit is the half sixteen provider tasks consume and nothing renders yet. `ObjectSourceForm`, `ObjectSourceOrigin`, `ObjectSourcePart` and `ObjectSourceDocument` join `src/lib/db/types.ts`, and `DatabaseProvider.readObjectSource` is added as the one OPTIONAL object method: two engines in the fleet have no kind with a definition text anywhere, so a required method would put an unreachable throw in each. `parts` is a non-empty tuple so a zero-part document is a compile error at every provider, and the refused arm carries no `text` key at all, so there is no path from a refusal to an editor buffer. Five derivations in `src/lib/db/object-kinds.ts`: `kindHasSource`, `isSourcePartUnavailable`, `SOURCE_CHARACTER_LIMIT`, `SOURCE_PART_LIMIT`, `sourceBoundTruncationReason` and `applySourceBound`. The last is hoisted here rather than written sixteen times, on the evidence that `comparePaths` was written four times before anyone owned it. All four types are published from `src/exports/types.ts`; every addition is new and additive, so nothing an external consumer compiles against moves. --- src/exports/types.ts | 4 ++ src/lib/db/object-kinds.ts | 72 +++++++++++++++++++- src/lib/db/types.ts | 103 +++++++++++++++++++++++++++++ tests/unit/db/object-kinds.test.ts | 82 ++++++++++++++++++++++- 4 files changed, 259 insertions(+), 2 deletions(-) diff --git a/src/exports/types.ts b/src/exports/types.ts index cd29e3e6..17c5c8d3 100644 --- a/src/exports/types.ts +++ b/src/exports/types.ts @@ -49,4 +49,8 @@ export type { KindCount, ObjectDetail, ObjectDetailBatch, + ObjectSourceForm, + ObjectSourceOrigin, + ObjectSourcePart, + ObjectSourceDocument, } from "../lib/db/types"; diff --git a/src/lib/db/object-kinds.ts b/src/lib/db/object-kinds.ts index 68db765f..30401a4d 100644 --- a/src/lib/db/object-kinds.ts +++ b/src/lib/db/object-kinds.ts @@ -5,7 +5,7 @@ * is answered here, in one place, so the defaults cannot drift: an absent * `acceptsRowWrites` reads as false in every caller because there is only one caller. */ -import type { KindCount, ObjectKindSpec, ProviderCapabilities } from "@/lib/db/types"; +import type { KindCount, ObjectKindSpec, ObjectSourcePart, ProviderCapabilities } from "@/lib/db/types"; /** * How many container levels this engine declares, as the tree models them. @@ -129,3 +129,73 @@ export function isCountSampled(count: KindCount): count is { readonly count: num export function callerBoundTruncationReason(limit: number): string { return `the bulk column read was bounded at ${limit} object${limit === 1 ? "" : "s"} by its caller`; } + +/** + * Whether THIS KIND has a readable definition (#789 Phase 2). + * + * Absent and undeclared both read as FALSE, and the name says the scope so a caller cannot + * inline the default. It is NOT conjoined with anything, for the same reason + * `kindAcceptsRowWrites` is not: the per-object question is a different one, and only the READ + * can answer it. Four kinds in the fleet are readable for some of their objects and not + * others, and this answers for the kind. + */ +export function kindHasSource(capabilities: ProviderCapabilities, id: string): boolean { + return findKind(capabilities, id)?.hasSource === true; +} + +/** + * The narrowing predicate for a refused part (#789 Phase 2). + * + * The `readonly` on every member is LOAD-BEARING and measured against TypeScript 6.0.3: a + * predicate written without it narrows the true branch and NOTHING on the false branch, so + * every caller is left holding the whole union with no `.text` on it. The one-property spelling + * `isCountUnavailable` uses does not compile here at all, because `ObjectSourcePart` has three + * required members on the refused arm, and that red build is the safe direction. + */ +export function isSourcePartUnavailable( + part: ObjectSourcePart, +): part is { readonly id: string; readonly label: string; readonly unavailable: string } { + return "unavailable" in part; +} + +/** The default per-part character bound the source route applies when a caller names none. */ +export const SOURCE_CHARACTER_LIMIT = 1_000_000; + +/** + * The most parts one document may carry before the route refuses it. + * + * The tuple type has no upper bound and the shipped maximum is two (an Oracle or MariaDB + * package), but the embedded seam takes its document from a HOST outside our compiler, so the + * real response size is `SOURCE_CHARACTER_LIMIT` times `parts.length` unless something bounds + * the count. Four times the largest shape any engine produces, so no correct provider can + * reach it. + */ +export const SOURCE_PART_LIMIT = 8; + +/** + * The ONE sentence a caller's source bound is reported with (#789 Phase 2). + * + * A function beside `callerBoundTruncationReason` rather than a reuse of it: the two bound + * different things and the existing sentence names objects. One place for the same reason that + * one records, which is that eleven implementers wrote three unrelated phrasings for one event + * before it was written down. + */ +export function sourceBoundTruncationReason(limit: number): string { + return `the source read was bounded at ${limit.toLocaleString("en-US")} characters by its caller`; +} + +/** + * One part's text under a caller's bound, with the mark the bound owes (#789 Phase 2). + * + * Hoisted here rather than written sixteen times, on the evidence that `comparePaths` was + * written four times before anyone owned it. An exact answer is NEVER marked, which is the + * rule `sampledFrom` already follows verbatim, because marking one teaches a reader to + * discount every mark. + */ +export function applySourceBound( + text: string, + limit: number | undefined, +): { readonly text: string; readonly truncated?: { readonly limit: number; readonly reason: string } } { + if (limit === undefined || text.length <= limit) return { text }; + return { text: text.slice(0, limit), truncated: { limit, reason: sourceBoundTruncationReason(limit) } }; +} diff --git a/src/lib/db/types.ts b/src/lib/db/types.ts index 686e4b6f..e2e76407 100644 --- a/src/lib/db/types.ts +++ b/src/lib/db/types.ts @@ -726,6 +726,31 @@ export interface DatabaseProvider { */ describeObjects(container: readonly string[], kind: string, limit?: number): Promise; + /** + * The definition text of ONE object, as a document of named parts (#789 Phase 2). + * + * OPTIONAL, unlike the five object methods above, and the asymmetry is argued rather than + * inherited. Those five are required because a provider that does not implement them + * answers nothing at all about what a database holds. A provider that does not implement + * this one answers everything about what the database holds and simply declares no + * source-bearing kind, which is the TRUE and measured state of `druid` and `libredb`: + * neither has a kind with a definition text anywhere, so a required method would put an + * unreachable throw in each, which is precisely the shape that got the 501 deleted. + * + * `kind` is required for the reason `describeObject`'s is: measured on MySQL, MariaDB and + * DuckDB, one name addresses more than one object of different kinds in one container, so a + * path alone reads the wrong object. + * + * `limit` bounds ONE PART's character count. Absent means unbounded. A provider may apply a + * bound of its own, and must then set `truncated` on the part it bounded and never on a part + * it read whole. + * + * The declaration and the method cannot disagree: `assertObjectSurface` asserts, in BOTH + * directions, that a provider declares a kind with `hasSource` exactly when it implements + * this method. + */ + readObjectSource?(path: readonly string[], kind: string, limit?: number): Promise; + /** * Get health and performance metrics */ @@ -1329,3 +1354,81 @@ export interface ObjectDetailBatch { /** Absent when every object of that kind in that container was described. */ readonly truncated?: { readonly limit: number; readonly reason: string }; } + +/** + * What this text IS, so a reader is never shown a fragment that looks like a statement (#789). + * + * CLOSED: two arms, both with producers in the shipped fleet. `complete` runs as given; + * `partial` is a body or a bare SELECT that does not. PostgreSQL's `pg_get_viewdef`, DuckDB's + * `macro_definition` and Couchbase's `definition.text` are the measured `partial` producers. + */ +export type ObjectSourceForm = "complete" | "partial"; + +/** + * Where this text came from, so a reader is never shown a reconstruction as an original (#789). + * + * CLOSED: three arms, each with at least one producer. `stored` is the author's own bytes + * (SQL Server modules, SQLite's `sqlite_schema.sql`); `regenerated` is the engine rebuilding + * from its catalog, which PostgreSQL documents as "a decompiled reconstruction, not the + * original text of the command"; `rendered` is a structured definition this product prints as + * JSON (a MongoDB view, a search pipeline or template). + */ +export type ObjectSourceOrigin = "stored" | "regenerated" | "rendered"; + +/** + * One text belonging to one object, or the engine's own reason there is none (#789). + * + * A UNION and not one shape with an optional `text`, for the reason `KindCount` is a union: a + * refusal and an empty answer are different facts, and a shape carrying `text?: string` makes + * them the same value at every call site. The refused arm has NO `text` key at all, so there is + * no path from a refusal to an editor buffer. That composition is what DBeaver gets wrong: + * measured in its source, an unreadable definition reaches a WRITABLE editor holding one + * comment line. + * + * `id` is provider-local. Core reads it as an identity WITHIN ONE DOCUMENT and for nothing + * else: the part switcher's selection key, and the Source tab's remembered selection. Core + * never compares it against a literal, never branches on it, and never carries it between two + * documents. + * + * `text` is never empty and never whitespace only. TypeScript cannot express that, so it is a + * runtime invariant asserted in `assertObjectSurface` for our own providers and in the client's + * shape check for a host's answer. Where an engine answers empty, the provider emits a REFUSAL + * carrying the engine's own fact instead. + */ +export type ObjectSourcePart = + | { + readonly id: string; + /** The engine's own word: "Package body", "Specification". Rendered as-is. */ + readonly label: string; + readonly text: string; + /** A Monaco language id the installed bundle registers. `plsql`, `tsql` and `cql` are not. */ + readonly language: string; + readonly form: ObjectSourceForm; + readonly origin: ObjectSourceOrigin; + readonly truncated?: { readonly limit: number; readonly reason: string }; + } + | { + readonly id: string; + readonly label: string; + /** The engine's own sentence, unprefixed, never a rewrite of it. */ + readonly unavailable: string; + }; + +/** + * One object's definition, as its provider reads it (#789). + * + * `parts` is a NON-EMPTY tuple, which makes a zero-part document a compile error at every + * provider: there is no shape in which the renderer is handed a document and has nothing to + * draw. Two spellings satisfy it and no third is accepted: an array literal, and + * `const parts: [ObjectSourcePart, ...ObjectSourcePart[]] = [first]` plus a conditional push. + * `rows.map(...)` does not, and casting past it defeats the whole invariant. + * + * More than one part is not a special case for one engine: an Oracle package and a MariaDB + * package are each ONE node over two texts, and core branches on `parts.length` and on nothing + * else. + */ +export interface ObjectSourceDocument { + readonly path: readonly string[]; + readonly kind: string; + readonly parts: readonly [ObjectSourcePart, ...ObjectSourcePart[]]; +} diff --git a/tests/unit/db/object-kinds.test.ts b/tests/unit/db/object-kinds.test.ts index 888a4191..b84a8151 100644 --- a/tests/unit/db/object-kinds.test.ts +++ b/tests/unit/db/object-kinds.test.ts @@ -1,5 +1,5 @@ import { describe, test, expect } from "bun:test"; -import type { ProviderCapabilities } from "@/lib/db/types"; +import type { ObjectSourcePart, ProviderCapabilities } from "@/lib/db/types"; import { containerDepth, declaredKinds, @@ -8,6 +8,12 @@ import { relationKindIds, isCountSampled, isCountUnavailable, + kindHasSource, + isSourcePartUnavailable, + applySourceBound, + sourceBoundTruncationReason, + SOURCE_CHARACTER_LIMIT, + SOURCE_PART_LIMIT, } from "@/lib/db/object-kinds"; const base = { queryLanguage: "sql" } as unknown as ProviderCapabilities; @@ -140,3 +146,77 @@ describe("isCountSampled", () => { expect(isCountSampled(count) ? count.sampledFrom : "").toBe("the first 1,000 keys of one SCAN walk"); }); }); + +describe("kindHasSource", () => { + test("an absent flag reads as false, so an undeclared kind never offers a Source tab", () => { + expect(kindHasSource(withKinds, "view")).toBe(false); + }); + + test("a kind this engine never declared reads as false rather than throwing", () => { + expect(kindHasSource(withKinds, "package")).toBe(false); + }); + + test("a declared flag reads as true", () => { + const caps = { + ...withKinds, + objectKinds: [{ id: "function", role: "routine", label: "F", labelPlural: "Fs", hasSource: true }], + } as unknown as ProviderCapabilities; + expect(kindHasSource(caps, "function")).toBe(true); + }); +}); + +describe("isSourcePartUnavailable", () => { + const refused: ObjectSourcePart = { id: "definition", label: "Definition", unavailable: "Encrypted." }; + const readable: ObjectSourcePart = { + id: "definition", + label: "Definition", + text: "SELECT 1", + language: "sql", + form: "complete", + origin: "stored", + }; + + test("narrows a refusal", () => { + expect(isSourcePartUnavailable(refused)).toBe(true); + }); + + test("narrows a readable part in the other direction, so the caller reaches text", () => { + expect(isSourcePartUnavailable(readable)).toBe(false); + if (isSourcePartUnavailable(readable)) throw new Error("unreachable"); + // This line is the whole point of the `readonly` spelling: without it the false branch + // still holds the union and `.text` does not exist. + expect(readable.text).toBe("SELECT 1"); + }); +}); + +describe("applySourceBound", () => { + test("an unbounded call marks nothing, because marking an exact answer teaches a reader to discount every mark", () => { + expect(applySourceBound("SELECT 1", undefined)).toEqual({ text: "SELECT 1" }); + }); + + test("a text that fits its bound is not marked either", () => { + expect(applySourceBound("SELECT 1", 8)).toEqual({ text: "SELECT 1" }); + }); + + test("a text over its bound is sliced and marked with the one sentence", () => { + const bounded = applySourceBound("SELECT 1", 6); + expect(bounded.text).toBe("SELECT"); + expect(bounded.truncated).toEqual({ limit: 6, reason: sourceBoundTruncationReason(6) }); + }); +}); + +describe("the source bounds", () => { + test("the character bound is the one number the route applies", () => { + expect(SOURCE_CHARACTER_LIMIT).toBe(1_000_000); + }); + + test("the part bound is four times the largest shape any engine in the fleet produces", () => { + expect(SOURCE_PART_LIMIT).toBe(8); + }); + + test("the bound sentence names the number and the caller", () => { + expect(sourceBoundTruncationReason(1_000_000)).toBe( + "the source read was bounded at 1,000,000 characters by its caller", + ); + }); +}); From e147e6d06cc81cc60435ddc2b840968deab3375b Mon Sep 17 00:00:00 2001 From: cevheri Date: Sat, 12 Sep 2026 20:52:53 +0300 Subject: [PATCH 02/94] feat(objects): assert the source contract, and read a Redis function library (#789) The conformance helper gains the source half, and Redis gains the method its declaration has promised since Phase 1: `redis.ts` holds the only kind in the fleet that declares `hasSource`, so the new pairing assertion is red against it the moment it lands and the two have to arrive together. What `assertObjectSurface` now asserts, each with a mutation that kills a test: - the pairing, in BOTH directions and outside every loop, so the fifteen providers that implement nothing are certified as strongly as the two that do; - a source-bearing kind the expectation never names is refused by name, one notch narrower than "names none": Oracle declares nine of them; - a document's path and kind are the ones it was asked for; - two parts of one document may not share an id; - a part's text is never empty and a refusal's sentence never blank; - a part's language agrees with the kind's declared `sourceLanguage`; - a caller's bound is honoured, reported with the one shared sentence and the right number, and NEVER reported on an unbounded read; - an absent object raises a `QueryError` naming the segment, with the same kind's successful read in the same run as its positive control. Redis reads `FUNCTION LIST LIBRARYNAME WITHCODE` and selects the entry whose `library_name` is BYTE-EQUAL to the last path segment. Measured on Redis 8.10.0: the library dictionary is case-sensitive, so `libredb_probe` and `LIBREDB_PROBE` coexist, while the `LIBRARYNAME` argument is a case-insensitive glob, so one lookup answers both and `reply[0]` would return the wrong library's Lua. The fixture now loads that pair. An absent library answers an empty array rather than an error, so emptiness is raised as absence; a denied read carries the server's own sentence unprefixed as the part's `unavailable`. Also measured and recorded in the provider doc: `redis-cli` reading from stdin does not skip a `#` line, it sends it as a command, so the fixture carries no comments. --- docker/redis-init/01-object-fixture.redis | 2 + docs/providers/redis.md | 75 ++- src/lib/db/providers/keyvalue/redis.ts | 125 +++- tests/helpers/object-surface-conformance.ts | 226 +++++++- tests/integration/db/redis-provider.test.ts | 258 ++++++++- .../db/object-surface-conformance.test.ts | 537 +++++++++++++++++- 6 files changed, 1211 insertions(+), 12 deletions(-) diff --git a/docker/redis-init/01-object-fixture.redis b/docker/redis-init/01-object-fixture.redis index 27c04d64..937b3132 100644 --- a/docker/redis-init/01-object-fixture.redis +++ b/docker/redis-init/01-object-fixture.redis @@ -12,3 +12,5 @@ DEL report:daily SET report:daily 42 SELECT 0 FUNCTION LOAD REPLACE "#!lua name=libredb_probe\nlocal function echo_key(keys, args)\n return redis.call('GET', keys[1])\nend\nlocal function ping(keys, args)\n return 'pong'\nend\nredis.register_function('libredb_echo_key', echo_key)\nredis.register_function('libredb_ping', ping)" +FUNCTION LOAD REPLACE "#!lua name=LIBREDB_PROBE\nlocal function upper_ping(keys, args)\n return 'PONG'\nend\nredis.register_function('LIBREDB_UPPER_PING', upper_ping)" +ACL SETUSER libredb_nofunction on >nofunction ~* +@all -function diff --git a/docs/providers/redis.md b/docs/providers/redis.md index 4aa7e04b..d310f6b6 100644 --- a/docs/providers/redis.md +++ b/docs/providers/redis.md @@ -837,6 +837,70 @@ The UTF-8-byte versus UTF-16-code-unit divergence Task 26a-2 measured on five SQ has nothing to bite on here for the same reason: there is no server-side sort to disagree with. +#### Object source (#789) + +`readObjectSource(path, kind, limit?)` answers ONE kind and the **declaration** says which. `function` +declares `hasSource` and `sourceLanguage: "lua"`; `keyspace` declares neither, because a key prefix is +a grouping this server derived from a bounded `SCAN` and nobody wrote a definition for it. That is the +`tablesAreDerivedGroupings` refusal carried into the object model rather than left behind with the +flag's old reader. The refusal is read off the declaration and never off the kind id, so a kind this +engine does not declare at all takes the same path and raises with the same sentence. + +The command is `FUNCTION LIST LIBRARYNAME WITHCODE`, sent once. It is server-scoped and takes +no database: measured, one `FUNCTION LOAD` is visible from every numbered database and `SELECT` does +not change what it answers. + +| Field | Value | Why | +|---|---|---| +| `id` | `definition` | one part, always: a library has one Lua text | +| `label` | `Definition` | rendered as-is | +| `language` | the kind's declared `sourceLanguage`, which is `lua` | `lua` IS a Monaco language id the installed 0.56.0 bundle registers, unlike `plsql`, `tsql` and `cql` | +| `form` | `complete` | the text runs as given: it is what `FUNCTION LOAD` was handed | +| `origin` | `stored` | the author's own bytes. Measured on Redis 8.10.0: `WITHCODE` answers the shebang line and the body exactly as they were loaded, with no reformatting | + +**The selection is BYTE-EQUAL, and that is the whole of the parser's reason to exist.** Measured on +Redis 8.10.0 against the committed fixture: + +``` +$ redis-cli FUNCTION LIST # the dictionary is CASE-SENSITIVE +library_name +libredb_probe +library_name +LIBREDB_PROBE +$ redis-cli FUNCTION LIST LIBRARYNAME libredb_probe # the argument is a CASE-INSENSITIVE glob +library_name +libredb_probe +library_name +LIBREDB_PROBE +``` + +One lookup for either name answers BOTH, so a reader taking `reply[0]` would hand back the other +library's Lua as this object's definition. The entry whose `library_name` is byte-equal to the last +path segment is the one read, and its `library_code` is found by walking the key/value pairs rather +than by position, the same rule the listing's parser records: the nested `functions` value is itself a +list of key/value lists, and RESP3 answers a map where there is no order at all. + +**An absent library RAISES.** Measured on Redis 8.10.0, +`FUNCTION LIST LIBRARYNAME no_such_library WITHCODE` answers an **empty array** and not an error, so +emptiness is absence here and a provider that returned a document would invent one. An entry that +matches by name and carries no `library_code`, or an empty one, takes the same arm: an empty text +would put an empty editor over a definition that was never read. + +**A refused read is the server's own sentence, unprefixed**, carried as the part's `unavailable` and +never as a text. Measured as the ACL user the fixture creates: + +``` +$ redis-cli --user libredb_nofunction --pass nofunction FUNCTION LIST LIBRARYNAME libredb_probe WITHCODE +NOPERM User libredb_nofunction has no permissions to run the 'function|list' command +``` + +KeyDB, DragonflyDB and Garnet have no `FUNCTION` command at all and each refuses in its own words (the +table in [§6.1](#61-the-object-surface-789) has them), so this path is reachable on three of the four +Redis-wire relatives this type id serves. + +**A caller's bound** cuts one part's text and reports itself through the one sentence every engine +uses, `the source read was bounded at characters by its caller`. An exact answer is never marked. + #### Reads go to the CONTAINER's database, never the session's Every object read opens its own short-lived connection with `db` set, rather than issuing `SELECT` on @@ -933,7 +997,7 @@ no control offers it. | `declaresForeignKeys` | `false` — Redis has no constraints at all, and the "tables" here are key prefixes this provider grouped rather than objects anyone declared | | `tablesAreDerivedGroupings` | `true` — the object surface SCANs a bounded slice of the keyspace and groups the real key names it found by their prefix, so a `user:*` row is this server's own summary and not a key any command can be given. The agent layer states this to a plan run, in one sentence, so a grounded run does not draft a command against a grouping. In the object tree it is what withholds Profile from a `keyspace` row ([§6.1](#61-the-object-surface-789)) | | `containerLevels` | one level, `schema`, labelled Database ([§6.1](#61-the-object-surface-789)) | -| `objectKinds` | `keyspace` (relation) and `function` (routine, `hasSource`, Lua). Three further candidates are absent rather than declared and zero ([§6.1](#61-the-object-surface-789)) | +| `objectKinds` | `keyspace` (relation) and `function` (routine, `hasSource`, `sourceLanguage: "lua"`). `function` is the only kind in this engine with a definition text, read through `FUNCTION LIST ... WITHCODE` ([§6.1](#61-the-object-surface-789)). Three further candidates are absent rather than declared and zero | | `supportsMaintenance` | `true` | | `maintenanceOperations` | `['analyze']` | | `supportsConnectionString` | `false` | @@ -1083,7 +1147,12 @@ docker exec -i libredb-redis redis-cli --no-raw < docker/redis-init/01-object-fi `redis-cli` reading from stdin uses ONE connection for the whole file, which is what makes the `SELECT 3` in the middle of it work; a per-line `redis-cli` loop would silently write every key into database 0. The file is idempotent: it `DEL`s the keys it is about to write and loads the function -library with `FUNCTION LOAD REPLACE`. +libraries with `FUNCTION LOAD REPLACE`. + +**The file carries no comments, and that is a constraint rather than a style.** Measured on +Redis 8.10.0: `redis-cli` reading from stdin does NOT skip a `#` line, it sends it as a command, and +the server answers ``ERR unknown command '#'``. Every explanation of what the fixture holds therefore +lives in the table below rather than beside the line. What it builds, and why each part is there: @@ -1093,6 +1162,8 @@ What it builds, and why each part is there: | db 0 | mixed value types under one prefix (string, hash, list) | the sampled `type` column is `string/hash`-shaped rather than uniform | | db 3 | `report:daily` | a key that exists in ONE database and nowhere else, so a provider reading the SESSION's database instead of the CONTAINER's is distinguishable from a correct one | | db 0 | function library `libredb_probe`, two registered functions | the `function` kind has an object, and `FUNCTION LIST WITHCODE` has source to answer | +| db 0 | a SECOND library `LIBREDB_PROBE`, differing from the first ONLY in case | `LIBRARYNAME` is a case-INSENSITIVE glob over a case-SENSITIVE dictionary, so one lookup answers both and a source read taking `reply[0]` shows the wrong library ([§6.1](#61-the-object-surface-789)) | +| server | ACL user `libredb_nofunction`, password `nofunction`, `-function` | a live principal for the source read's refusal pane | To measure a cluster-mode container, which is the only deployment where the database count is not 16: diff --git a/src/lib/db/providers/keyvalue/redis.ts b/src/lib/db/providers/keyvalue/redis.ts index 59006a6d..20c3b3d4 100644 --- a/src/lib/db/providers/keyvalue/redis.ts +++ b/src/lib/db/providers/keyvalue/redis.ts @@ -16,7 +16,13 @@ import Redis, { type RedisOptions } from "ioredis"; import { BaseDatabaseProvider } from "../../base-provider"; -import { callerBoundTruncationReason, containerDepth, declaredKinds, findKind } from "../../object-kinds"; +import { + applySourceBound, + callerBoundTruncationReason, + containerDepth, + declaredKinds, + findKind, +} from "../../object-kinds"; import { comparePaths } from "../../object-path"; import { type DatabaseConnection, @@ -44,6 +50,7 @@ import { type ObjectDetail, type ObjectDetailBatch, type ObjectKindSpec, + type ObjectSourceDocument, } from "../../types"; import { DatabaseConfigError, QueryError, ConnectionError } from "../../errors"; @@ -339,6 +346,39 @@ function parseFunctionLibraries(reply: unknown): string[] { return names; } +/** + * One library's `library_code` out of a `FUNCTION LIST ... WITHCODE` reply, selected + * BYTE-EQUAL (#789 Phase 2). + * + + * The selection is the whole of this function's reason to exist. MEASURED on redis 8.10.0 + * against the committed fixture: the library dictionary is CASE-SENSITIVE, so `libredb_probe` + * and `LIBREDB_PROBE` coexist, while the `LIBRARYNAME` argument is a CASE-INSENSITIVE glob, so + * ONE lookup for either name answers BOTH. `reply[0]` would therefore hand back the other + * library's Lua as this object's definition, and `docker/redis-init/01-object-fixture.redis` + * holds that pair for exactly this reason. Reply order is not part of the protocol contract: + * RESP3 answers a map, where there is no order at all. + * + * The pairs are walked rather than indexed, the same rule `parseFunctionLibraries` records: + * the nested `functions` value is itself a list of key/value lists, so a parser reading + * positions takes a field name for a library name the moment the server adds a field. + */ +function parseFunctionLibraryCode(reply: unknown, name: string): string | undefined { + for (const entry of Array.isArray(reply) ? reply : []) { + if (!Array.isArray(entry)) continue; + let matched = false; + let code: string | undefined; + for (let index = 0; index + 1 < entry.length; index += 2) { + const key = String(entry[index]); + const value = entry[index + 1]; + if (key === "library_name" && value === name) matched = true; + if (key === "library_code" && typeof value === "string") code = value; + } + if (matched) return code; + } + return undefined; +} + // ============================================================================ // Redis Provider // ============================================================================ @@ -1168,6 +1208,89 @@ export class RedisProvider extends BaseDatabaseProvider { }); } + /** + * `FUNCTION LIST LIBRARYNAME WITHCODE`, as its own method (#789 Phase 2). + * + * A method rather than an inline call so the refusal arm of `readObjectSource` can be + * driven without reaching into ioredis, and so the command text has one writer. It is + * SERVER-SCOPED and takes no database: measured on redis 8.10.0, one `FUNCTION LOAD` is + * visible from every numbered database and `SELECT` does not change what `FUNCTION LIST` + * answers, which is the same fact `listObjects` records for the listing. + */ + private async callFunctionList(name: string): Promise { + // eslint-disable-next-line @typescript-eslint/no-explicit-any + return await (this.client as any).call("FUNCTION", "LIST", "LIBRARYNAME", name, "WITHCODE"); + } + + /** + * A function library's Lua source (#789 Phase 2). + * + * ONE kind can answer here and the DECLARATION says which: `function` declares `hasSource` + * and `keyspace` does not, because a key prefix is a grouping this server derived from a + * bounded `SCAN` and nobody wrote a definition for it. That is the + * `tablesAreDerivedGroupings` refusal carried into the object model rather than left behind + * with the flag's old reader. The refusal is read off the declaration and never off the + * kind id, so a kind this engine does not declare at all takes the same path. + * + * A library the server does not hold RAISES. It has to: measured on redis 8.10.0, + * `FUNCTION LIST LIBRARYNAME no_such_library WITHCODE` answers an EMPTY ARRAY and not an error, so + * emptiness is absence here and a provider that returned a document would invent one. A + * matching entry carrying no `library_code` takes the same arm, because an empty text would + * put an empty editor over a definition that was never read. + * + * A refusal is the server's own sentence, unprefixed. KeyDB, DragonflyDB and Garnet have no + * `FUNCTION` command at all and each refuses in its own words (all measured 2026-09-11), so + * this path is reachable on three of the four Redis-wire relatives this type id serves. + * + * The name is `path[path.length - 1]` and never `path[1]`: standing ruling 5g, and the + * integration suite pins it by swapping a two-level declaration in. + */ + public async readObjectSource(path: readonly string[], kind: string, limit?: number): Promise { + this.ensureConnected(); + const capabilities = this.getCapabilities(); + const spec = findKind(capabilities, kind); + if (spec?.hasSource !== true) { + throw new QueryError(`Redis declares no readable source for the kind "${kind}"`, "redis"); + } + const name = path[path.length - 1]; + let reply: unknown; + try { + reply = await this.callFunctionList(name); + } catch (error) { + return { + path: [...path], + kind, + parts: [ + { + id: "definition", + label: "Definition", + unavailable: error instanceof Error ? error.message : String(error), + }, + ], + }; + } + const code = parseFunctionLibraryCode(reply, name); + if (code === undefined || code.trim() === "") { + throw new QueryError(`Redis holds no function library called "${name}"`, "redis"); + } + const bounded = applySourceBound(code, limit); + return { + path: [...path], + kind, + parts: [ + { + id: "definition", + label: "Definition", + text: bounded.text, + language: spec.sourceLanguage ?? "lua", + form: "complete", + origin: "stored", + ...(bounded.truncated === undefined ? {} : { truncated: bounded.truncated }), + }, + ], + }; + } + /** * Columns for EVERY object of one kind in one database, from ONE walk (#789). * diff --git a/tests/helpers/object-surface-conformance.ts b/tests/helpers/object-surface-conformance.ts index 4fa827bf..93875c8e 100644 --- a/tests/helpers/object-surface-conformance.ts +++ b/tests/helpers/object-surface-conformance.ts @@ -59,8 +59,23 @@ * which names neither the kind nor the expectation. */ import { expect } from "bun:test"; -import type { DatabaseObject, DatabaseProvider, KindCount, ObjectDetailBatch } from "@/lib/db/types"; -import { callerBoundTruncationReason, declaredKinds, isCountUnavailable, relationKindIds } from "@/lib/db/object-kinds"; +import { QueryError } from "@/lib/db/errors"; +import type { + DatabaseObject, + DatabaseProvider, + KindCount, + ObjectDetailBatch, + ObjectSourceDocument, +} from "@/lib/db/types"; +import { + callerBoundTruncationReason, + declaredKinds, + findKind, + isCountUnavailable, + isSourcePartUnavailable, + relationKindIds, + sourceBoundTruncationReason, +} from "@/lib/db/object-kinds"; // The SAME key the providers and the joins use. A helper keying paths its own way could // certify a provider whose own reader disagrees with it about what one path is. import { pathKey } from "@/lib/db/object-path"; @@ -84,6 +99,15 @@ export interface ObjectSurfaceExpectation { readonly container?: readonly string[]; readonly kinds: Readonly>; readonly sampleObject: { readonly path: readonly string[]; readonly kind: string }; + /** + * A path whose last segment names nothing, and the source-bearing kind to ask it under. + * + * AUTHORED rather than found, which is the deliberate asymmetry with `sampleObject`: only + * the test author knows what is illegal or impossible on that engine, and there is by + * definition no listing that produced it. Required whenever the provider declares a + * source-bearing kind, and the helper throws by name when it is missing. + */ + readonly absentSource?: { readonly path: readonly string[]; readonly kind: string }; } function startsWith(path: readonly string[], prefix: readonly string[]): boolean { @@ -194,6 +218,7 @@ export async function assertObjectSurface( expect(detail.path).toEqual([...sample.path]); await assertBulkColumnRead(provider, container, listings); + await assertSourceSurface(provider, expected, listings); } /** @@ -372,3 +397,200 @@ async function assertBulkColumnRead( ); } } + +/** + * The optional sixth method, checked against the provider's OWN listing (#789 Phase 2). + * + * Every path driven here is one the PROVIDER produced, for the reason `assertBulkColumnRead` + * records: the only thing that makes an assertion about a read non-vacuous is that it compares + * two answers the provider gave, never one the test author typed. The single exception is + * `absentSource`, which cannot be found by definition, and which therefore carries a POSITIVE + * CONTROL in the same helper: the loop above must already have read a document for that kind, + * so a rejection cannot be a connection failure or a bad bind. + * + * The zero-iteration case of each loop is what the throws guard: + * + * - the PAIRING is outside every loop, so the fifteen providers that implement nothing are + * certified exactly as strongly as the two that implement something: `false === false` is + * an assertion too, and it is the only thing standing between a declaration and a method + * that disagree; + * - a source-bearing kind the expectation never NAMES is refused by name, one notch narrower + * than "named none". Oracle declares nine of them and an expectation naming one would + * silence a "none" guard while eight kinds went unread; + * - a document of nothing but refusals answers no readable part, so the bound probe below + * would never run and every bound assertion would be vacuous; + * - a definition under two characters cannot be bounded distinguishably, which is the same + * bar `richest.count < 2` sets for the bulk read. + */ +async function assertSourceSurface( + provider: DatabaseProvider, + expected: ObjectSurfaceExpectation, + listings: ReadonlyMap, +): Promise { + const capabilities = provider.getCapabilities(); + const sourceKinds = declaredKinds(capabilities).filter((kind) => kind.hasSource === true); + const read = provider.readObjectSource; + + // The pairing, unconditional and outside every loop. A declaration with no method behind it + // surfaces as a 400 at runtime rather than a red build, and this is what catches it. + if (sourceKinds.length > 0 !== (typeof read === "function")) { + throw new Error( + `${provider.type} declares ${sourceKinds.length} source-bearing kind(s) and ` + + `${typeof read === "function" ? "implements readObjectSource" : "does not implement readObjectSource"}`, + ); + } + if (read === undefined) return; + + const absent = expected.absentSource; + if (absent === undefined) { + throw new Error( + "the provider declares a source-bearing kind and the expectation names no absentSource, so the " + + "absence raise is never driven", + ); + } + + // NAMED rather than counted above zero. A kind the expectation names at zero is an + // acknowledged absence that the count assertion already pins, and a fixture holding none of + // a declared kind is a legitimate state ruling 4 describes. A kind the expectation OMITS is + // the silent hole, and it is the one refused here. + const unexercised = sourceKinds.filter((kind) => !Object.hasOwn(expected.kinds, kind.id)).map((kind) => kind.id); + if (unexercised.length > 0) { + throw new Error( + `${provider.type} declares source-bearing kinds the expectation never exercised (${unexercised.join(", ")}), ` + + "so every source assertion for them is vacuous", + ); + } + + const wanted = new Set(sourceKinds.filter((kind) => (expected.kinds[kind.id] ?? 0) > 0).map((kind) => kind.id)); + const entered = new Set(); + let longest: { kind: string; path: readonly string[]; length: number } | undefined; + + // `listings` and not `wanted` drives the walk, because `listings` is what the provider + // actually produced. Every entry it holds for a kind the expectation counts above zero is + // known non-empty: the listing loop threw otherwise. + for (const [kindId, objects] of listings) { + if (!wanted.has(kindId)) continue; + const object = objects[0]; + const document = await read.call(provider, object.path, kindId); + entered.add(kindId); + assertSourceDocument(document, object, kindId, findKind(capabilities, kindId)?.sourceLanguage); + for (const part of document.parts) { + if (isSourcePartUnavailable(part)) continue; + // The escape hatch a bounded probe would otherwise leave open: a provider could answer + // short on an unbounded call and wave the flag at it. A bound of its OWN stays + // certifiable; the CALLER's bound is not, because no caller passed one. + if ( + part.truncated !== undefined && + part.truncated.reason.includes(sourceBoundTruncationReason(part.truncated.limit)) + ) { + throw new Error( + `readObjectSource("${kindId}") was called with no limit and reported one: "${part.truncated.reason}"`, + ); + } + if (longest === undefined || part.text.length > longest.length) { + longest = { kind: kindId, path: object.path, length: part.text.length }; + } + } + } + + if (longest === undefined) { + throw new Error("no source-bearing kind answered a readable part, so every source assertion is vacuous"); + } + // A bounded probe proves nothing unless the UNBOUNDED answer is longer than the bound. + if (longest.length < 2) { + throw new Error( + `the longest definition read is ${longest.length} character(s), so a bound cannot be told from no bound`, + ); + } + + const probe = Math.floor(longest.length / 2); + const bounded = await read.call(provider, longest.path, longest.kind, probe); + let marked = false; + for (const part of bounded.parts) { + if (isSourcePartUnavailable(part)) continue; + if (part.text.length > probe) { + throw new Error(`readObjectSource("${longest.kind}", limit ${probe}) returned ${part.text.length} characters`); + } + if (part.truncated === undefined) continue; + expect(part.truncated.limit).toBe(probe); + const sentence = sourceBoundTruncationReason(probe); + if (!part.truncated.reason.includes(sentence)) { + throw new Error( + `readObjectSource("${longest.kind}", limit ${probe}) reported "${part.truncated.reason}", which does ` + + `not carry the one sentence a caller's bound is reported with: "${sentence}"`, + ); + } + marked = true; + } + if (!marked) { + throw new Error( + `readObjectSource("${longest.kind}", limit ${probe}) bounded a ${longest.length}-character definition and ` + + "reported no truncation", + ); + } + + // The absence, with its positive control: the loop above already resolved a document for + // this kind, so a rejection here cannot be a connection failure or a bad bind. + if (!entered.has(absent.kind)) { + throw new Error( + `absentSource names the kind "${absent.kind}", which the source loop never read, so its rejection has no ` + + "control", + ); + } + let raised: unknown; + try { + await read.call(provider, absent.path, absent.kind); + } catch (error) { + raised = error; + } + if (!(raised instanceof QueryError)) { + throw new Error( + `readObjectSource did not raise for ${JSON.stringify(absent.path)}; it answered ` + + `${raised === undefined ? "a document" : String(raised)}`, + ); + } + const segment = absent.path[absent.path.length - 1]; + if (!raised.message.includes(segment)) { + throw new Error( + `readObjectSource raised for ${JSON.stringify(absent.path)} without naming "${segment}": "${raised.message}"`, + ); + } +} + +/** + * One document's own shape. + * + * What is NOT checked here, and why: a part carrying BOTH `text` and `unavailable` is a + * compile error for our own providers, so a check for it here would be a line no test can + * reach. A HOST can produce one, and the client's shape check is where that is caught. + */ +function assertSourceDocument( + document: ObjectSourceDocument, + object: DatabaseObject, + kindId: string, + declaredLanguage: string | undefined, +): void { + expect(document.path).toEqual([...object.path]); + expect(document.kind).toBe(kindId); + const ids = new Set(); + for (const part of document.parts) { + if (ids.has(part.id)) { + throw new Error(`two parts of ${JSON.stringify(object.path)} share the id "${part.id}"`); + } + ids.add(part.id); + if (isSourcePartUnavailable(part)) { + if (part.unavailable.trim() === "") { + throw new Error(`readObjectSource("${kindId}") answered a refusal with no sentence a person can read`); + } + continue; + } + if (part.text.trim() === "") { + throw new Error(`readObjectSource("${kindId}") answered a part with no text, which is not a definition`); + } + if (declaredLanguage !== undefined && part.language !== declaredLanguage) { + throw new Error( + `kind "${kindId}" declares sourceLanguage "${declaredLanguage}" and the part carries "${part.language}"`, + ); + } + } +} diff --git a/tests/integration/db/redis-provider.test.ts b/tests/integration/db/redis-provider.test.ts index aecf1bc1..66b34c42 100644 --- a/tests/integration/db/redis-provider.test.ts +++ b/tests/integration/db/redis-provider.test.ts @@ -6,6 +6,7 @@ */ import { describe, test, expect, beforeEach, afterEach, mock, spyOn } from "bun:test"; import { assertObjectSurface } from "../../helpers/object-surface-conformance"; +import { isSourcePartUnavailable, sourceBoundTruncationReason } from "@/lib/db/object-kinds"; import type { DatabaseConnection } from "@/lib/types"; import { generateTableQuery, generateSelectQuery } from "@/lib/query-generators"; @@ -127,6 +128,17 @@ const MOCK_FUNCTION_LIST: unknown[] = [ ["name", "libredb_echo_key", "description", null, "flags", []], ], ], + // The case-variant sibling `docker/redis-init/01-object-fixture.redis` now loads. The + // library dictionary is CASE-SENSITIVE: measured on redis 8.10.0, `libredb_probe` and + // `LIBREDB_PROBE` coexist and `FUNCTION LIST` answers both, in this order. + [ + "library_name", + "LIBREDB_PROBE", + "engine", + "LUA", + "functions", + [["name", "LIBREDB_UPPER_PING", "description", null, "flags", []]], + ], ]; /** @@ -137,6 +149,79 @@ const MOCK_FUNCTION_LIST: unknown[] = [ */ let functionListReply: unknown[] = MOCK_FUNCTION_LIST; +/** + * The library's Lua source, byte for byte what `docker/redis-init/01-object-fixture.redis` + * loads. MEASURED on redis 8.10.0: `FUNCTION LIST ... WITHCODE` answers the shebang line and + * the body exactly as they were given to `FUNCTION LOAD`, with no reformatting. + */ +const FIXTURE_LIBRARY_CODE = [ + "#!lua name=libredb_probe", + "local function echo_key(keys, args)", + " return redis.call('GET', keys[1])", + "end", + "local function ping(keys, args)", + " return 'pong'", + "end", + "redis.register_function('libredb_echo_key', echo_key)", + "redis.register_function('libredb_ping', ping)", +].join("\n"); + +/** The SECOND library, differing from the first ONLY in case. See the reply below. */ +const FIXTURE_UPPER_LIBRARY_CODE = [ + "#!lua name=LIBREDB_PROBE", + "local function upper_ping(keys, args)", + " return 'PONG'", + "end", + "redis.register_function('LIBREDB_UPPER_PING', upper_ping)", +].join("\n"); + +/** + * What `FUNCTION LIST LIBRARYNAME libredb_probe WITHCODE` answers, and the reason the reply + * carries TWO libraries. + * + * MEASURED on redis 8.10.0 against the committed fixture: the library dictionary is + * CASE-SENSITIVE, so `libredb_probe` and `LIBREDB_PROBE` coexist, while `LIBRARYNAME` is a + * CASE-INSENSITIVE glob, so ONE lookup for either name answers BOTH. A reader taking + * `reply[0]` would hand back the other library's Lua as this object's definition. + * + * The wrong library is FIRST here and the live server happened to answer the exact match + * first. That is deliberate: reply order is not part of the protocol contract, RESP3 answers + * a map with no order at all, and a mock that reproduced the lucky order would certify a + * parser that indexes. + */ +const MOCK_FUNCTION_WITHCODE: unknown[] = [ + [ + "library_name", + "LIBREDB_PROBE", + "engine", + "LUA", + "functions", + [["name", "LIBREDB_UPPER_PING", "description", null, "flags", []]], + "library_code", + FIXTURE_UPPER_LIBRARY_CODE, + ], + [ + "library_name", + "libredb_probe", + "engine", + "LUA", + "functions", + [ + ["name", "libredb_ping", "description", null, "flags", []], + ["name", "libredb_echo_key", "description", null, "flags", []], + ], + "library_code", + FIXTURE_LIBRARY_CODE, + ], +]; + +/** + * What a `WITHCODE` read answers, reset before each object-surface test. A test empties it to + * stand for a library the server does not hold: measured on redis 8.10.0, + * `FUNCTION LIST LIBRARYNAME no_such_library` answers an EMPTY ARRAY rather than an error. + */ +let functionWithCodeReply: unknown[] = MOCK_FUNCTION_WITHCODE; + /** * When set, `FUNCTION LIST` rejects with this sentence. Three of the four Redis-wire * relatives do exactly that and each says it differently (all measured 2026-09-11): @@ -255,6 +340,10 @@ mock.module("ioredis", () => { if (cmd === "CONFIG") return databasesReply; if (cmd === "FUNCTION") { if (functionRefusal !== null) throw new Error(functionRefusal); + // WITHCODE is the source read and LIST without it is the listing. The two answer + // different shapes on a real server and the mock has to as well, or a provider + // reading `library_code` off the listing reply would pass. + if (args.some((arg) => arg.toUpperCase() === "WITHCODE")) return functionWithCodeReply; return functionListReply; } if (cmd in mockCallResults) { @@ -1226,6 +1315,7 @@ describe("RedisProvider", () => { beforeEach(async () => { databasesReply = ["databases", "16"]; functionListReply = MOCK_FUNCTION_LIST; + functionWithCodeReply = MOCK_FUNCTION_WITHCODE; functionRefusal = null; scanRefusal = null; scanOverflows = false; @@ -1259,11 +1349,162 @@ describe("RedisProvider", () => { test("satisfies the object-surface contract", async () => { await assertObjectSurface(provider, { containers: Array.from({ length: 16 }, (_, index) => [String(index)]), - kinds: { keyspace: 2, function: 1 }, + kinds: { keyspace: 2, function: 2 }, sampleObject: { path: ["0", "user:*"], kind: "keyspace" }, + absentSource: { path: ["0", "no_such_library"], kind: "function" }, }); }); + test("reads a function library's Lua source, selecting the byte-equal name", async () => { + const document = await provider.readObjectSource!(["0", "libredb_probe"], "function"); + + expect(document.path).toEqual(["0", "libredb_probe"]); + expect(document.kind).toBe("function"); + expect(document.parts).toHaveLength(1); + const [part] = document.parts; + if (isSourcePartUnavailable(part)) throw new Error("the fixture library is readable"); + expect(part.id).toBe("definition"); + expect(part.label).toBe("Definition"); + expect(part.language).toBe("lua"); + expect(part.form).toBe("complete"); + expect(part.origin).toBe("stored"); + expect(part.truncated).toBeUndefined(); + expect(part.text).toBe(FIXTURE_LIBRARY_CODE); + // The case pair is why the selection is byte-equal: FUNCTION LIST LIBRARYNAME + // glob-matches case-INSENSITIVELY over a case-SENSITIVE dictionary, so this reply + // carries two libraries and the FIRST of them is the wrong one. + expect(part.text).not.toContain("LIBREDB_UPPER_PING"); + expect(commandsSent()).toContain("FUNCTION LIST LIBRARYNAME libredb_probe WITHCODE"); + }); + + test("bounds one part at the caller's limit and reports the one shared sentence", async () => { + const document = await provider.readObjectSource!(["0", "libredb_probe"], "function", 24); + const [part] = document.parts; + if (isSourcePartUnavailable(part)) throw new Error("the fixture library is readable"); + + expect(part.text).toBe(FIXTURE_LIBRARY_CODE.slice(0, 24)); + expect(part.truncated).toEqual({ limit: 24, reason: sourceBoundTruncationReason(24) }); + }); + + test("raises for a library the server does not hold, because an empty reply is absence", async () => { + // Measured on redis 8.10.0: FUNCTION LIST LIBRARYNAME no_such_library WITHCODE answers + // an EMPTY ARRAY rather than an error, so emptiness is absence to whatever asks. + functionWithCodeReply = []; + await expect(provider.readObjectSource!(["0", "no_such_library"], "function")).rejects.toThrow(/no_such_library/); + }); + + test("raises for a kind that declares no source, so keyspace never reaches an editor", async () => { + // The `tablesAreDerivedGroupings` refusal carried into the object model: a key prefix + // is a grouping this server derived from a bounded SCAN and nobody wrote a definition + // for it. The refusal is driven off the DECLARATION and never off the kind id. + await expect(provider.readObjectSource!(["0", "user:*"], "keyspace")).rejects.toThrow( + /no readable source for the kind "keyspace"/, + ); + }); + + test("raises for a kind this engine does not declare at all", async () => { + await expect(provider.readObjectSource!(["0", "x"], "procedure")).rejects.toThrow( + /no readable source for the kind "procedure"/, + ); + }); + + test("carries the server's own refusal when the ACL denies FUNCTION", async () => { + // MEASURED on redis 8.10.0 as the ACL user `libredb_nofunction` the fixture creates. + functionRefusal = "NOPERM User libredb_nofunction has no permissions to run the 'function|list' command"; + const document = await provider.readObjectSource!(["0", "libredb_probe"], "function"); + const [part] = document.parts; + + if (!isSourcePartUnavailable(part)) throw new Error("a denied read is a refusal"); + expect(part.unavailable).toBe( + "NOPERM User libredb_nofunction has no permissions to run the 'function|list' command", + ); + expect("text" in part).toBe(false); + }); + + test("a library whose code the server withheld is absence rather than an empty definition", async () => { + // The entry matches by name and carries no `library_code`. Answering a part with an + // empty text would put an empty editor over a definition that was never read, which is + // the DBeaver shape this contract exists to make unrepresentable. + functionWithCodeReply = [["library_name", "libredb_probe", "engine", "LUA"]]; + await expect(provider.readObjectSource!(["0", "libredb_probe"], "function")).rejects.toThrow(/libredb_probe/); + }); + + test("a library the server answers with an EMPTY code is absence, not an empty definition", async () => { + // Distinct from the entry that carries no `library_code` at all: this one carries the + // key with nothing in it, which is the shape a reader would most easily hand to an + // editor as a blank buffer. + functionWithCodeReply = [["library_name", "libredb_probe", "engine", "LUA", "library_code", " "]]; + await expect(provider.readObjectSource!(["0", "libredb_probe"], "function")).rejects.toThrow( + /Redis holds no function library called "libredb_probe"/, + ); + }); + + test("the part's language is the kind's DECLARED sourceLanguage, not a literal", async () => { + spyOn(provider, "getCapabilities").mockReturnValue({ + ...provider.getCapabilities(), + objectKinds: [ + { id: "keyspace", role: "relation", label: "Key Pattern", labelPlural: "Key Patterns" }, + { + id: "function", + role: "routine", + label: "Function Library", + labelPlural: "Function Libraries", + hasSource: true, + sourceLanguage: "luau", + }, + ], + } as ReturnType); + + const document = await provider.readObjectSource!(["0", "libredb_probe"], "function"); + const [part] = document.parts; + if (isSourcePartUnavailable(part)) throw new Error("the fixture library is readable"); + expect(part.language).toBe("luau"); + }); + + test("a source-bearing kind that declares no language falls back to this engine's own", async () => { + spyOn(provider, "getCapabilities").mockReturnValue({ + ...provider.getCapabilities(), + objectKinds: [ + { id: "keyspace", role: "relation", label: "Key Pattern", labelPlural: "Key Patterns" }, + { + id: "function", + role: "routine", + label: "Function Library", + labelPlural: "Function Libraries", + hasSource: true, + }, + ], + } as ReturnType); + + const document = await provider.readObjectSource!(["0", "libredb_probe"], "function"); + const [part] = document.parts; + if (isSourcePartUnavailable(part)) throw new Error("the fixture library is readable"); + expect(part.language).toBe("lua"); + }); + + /** + * Standing ruling 5g, pinned on a one-level engine (#789). + * + * The declaration is swapped for a two-level one and the read is driven all the way to + * the NAME it selects by. A provider taking `path[1]` is behaviour-identical at depth 1 + * and silently wrong here, and a provider hardcoding the depth would refuse a path it + * must accept. + */ + test("the library name comes from the declared depth, not from a fixed position", async () => { + spyOn(provider, "getCapabilities").mockReturnValue({ + ...provider.getCapabilities(), + containerLevels: [ + { id: "catalog", label: "Catalog", labelPlural: "Catalogs" }, + { id: "schema", label: "Database", labelPlural: "Databases" }, + ], + } as ReturnType); + + const document = await provider.readObjectSource!(["main", "0", "libredb_probe"], "function"); + + expect(document.path).toEqual(["main", "0", "libredb_probe"]); + expect(commandsSent()).toContain("FUNCTION LIST LIBRARYNAME libredb_probe WITHCODE"); + }); + test("the container list is the deployment's own database count, not a constant 16", async () => { databasesReply = ["databases", "1"]; const containers = await provider.listContainers(); @@ -1293,13 +1534,13 @@ describe("RedisProvider", () => { test("counts both kinds, seeded at zero before either read answers", async () => { const counts = await provider.countObjects(["0"]); - expect(counts).toEqual({ keyspace: { count: 2 }, function: { count: 1 } }); + expect(counts).toEqual({ keyspace: { count: 2 }, function: { count: 2 } }); expect(commandsSent()).toContain("FUNCTION LIST"); }); test("an empty database counts zero rather than losing its folders", async () => { const counts = await provider.countObjects(["7"]); - expect(counts).toEqual({ keyspace: { count: 0 }, function: { count: 1 } }); + expect(counts).toEqual({ keyspace: { count: 0 }, function: { count: 2 } }); }); // Measured on three of the four Redis-wire relatives, each with its own sentence. @@ -1324,7 +1565,7 @@ describe("RedisProvider", () => { expect(counts).toEqual({ keyspace: { count: 3, sampledFrom: "the first 1,000 keys of one SCAN walk" }, - function: { count: 1 }, + function: { count: 2 }, }); }); @@ -1345,7 +1586,7 @@ describe("RedisProvider", () => { expect(counts).toEqual({ keyspace: { unavailable: "NOPERM this user has no permissions to run the 'scan' command" }, - function: { count: 1 }, + function: { count: 2 }, }); }); @@ -1370,7 +1611,12 @@ describe("RedisProvider", () => { test("lists function libraries by their library_name, not by position", async () => { const objects = await provider.listObjects(["0"], "function"); - expect(objects).toEqual([{ path: ["0", "libredb_probe"], name: "libredb_probe", kind: "function" }]); + // Sorted by `comparePaths`, which orders the segments: "LIBREDB_PROBE" precedes + // "libredb_probe" because the code units do. + expect(objects).toEqual([ + { path: ["0", "LIBREDB_PROBE"], name: "LIBREDB_PROBE", kind: "function" }, + { path: ["0", "libredb_probe"], name: "libredb_probe", kind: "function" }, + ]); expect(commandsSent()).toContain("FUNCTION LIST"); }); diff --git a/tests/unit/db/object-surface-conformance.test.ts b/tests/unit/db/object-surface-conformance.test.ts index 69ff572a..16de2bca 100644 --- a/tests/unit/db/object-surface-conformance.test.ts +++ b/tests/unit/db/object-surface-conformance.test.ts @@ -1,5 +1,6 @@ import { describe, test, expect } from "bun:test"; -import { callerBoundTruncationReason } from "@/lib/db/object-kinds"; +import { QueryError } from "@/lib/db/errors"; +import { applySourceBound, callerBoundTruncationReason, sourceBoundTruncationReason } from "@/lib/db/object-kinds"; import { assertObjectSurface } from "../../helpers/object-surface-conformance"; function fakeProvider(overrides: Record = {}) { @@ -849,3 +850,537 @@ describe("assertObjectSurface and the bulk column read", () => { ]); }); }); + +/** + * The source half (#789 Phase 2). + * + * Every test here drives a deliberately WRONG provider double and asserts the helper + * throws, because a correct provider passes a weak helper: "conformance passes" is + * evidence about the provider and never about the helper. + */ +describe("assertObjectSurface and the object source read", () => { + const listed: Record = { + table: [ + { path: ["app", "orders"], name: "orders", kind: "table" }, + { path: ["app", "products"], name: "products", kind: "table" }, + ], + view: [{ path: ["app", "order_summary"], name: "order_summary", kind: "view" }], + function: [ + { path: ["app", "order_total(integer)"], name: "order_total", kind: "function" }, + { path: ["app", "order_tax(integer)"], name: "order_tax", kind: "function" }, + ], + }; + + const readable = "SELECT 1 FROM orders"; + const absentName = "no_such_routine(integer)"; + + // Every positive double raises for the absent path, because the helper drives the absence + // arm on every provider it certifies. A double that answered a document there would fail + // for the one reason the test is not about. + function raiseIfAbsent(path: readonly string[]): void { + if (path[path.length - 1] === absentName) { + throw new QueryError(`no routine called "${absentName}" in schema app`, "postgres"); + } + } + + function capabilities(kindOverrides: Record = {}) { + return { + queryLanguage: "sql", + containerLevels: [{ id: "schema", label: "Schema", labelPlural: "Schemas" }], + objectKinds: [ + { id: "table", role: "relation", label: "Table", labelPlural: "Tables" }, + { id: "view", role: "relation", label: "View", labelPlural: "Views" }, + { + id: "function", + role: "routine", + label: "Function", + labelPlural: "Functions", + hasSource: true, + sourceLanguage: "sql", + ...kindOverrides, + }, + ], + }; + } + + function sourceProvider(overrides: Record = {}) { + return fakeProvider({ + type: "postgres", + getCapabilities: () => capabilities(), + countObjects: async () => ({ table: { count: 2 }, view: { count: 1 }, function: { count: 2 } }), + listObjects: async (_c: readonly string[], kind: string) => listed[kind] ?? [], + readObjectSource: async (path: readonly string[], kind: string, limit?: number) => { + raiseIfAbsent(path); + return { + path, + kind, + parts: [ + { + id: "definition", + label: "Definition", + ...applySourceBound(readable, limit), + language: "sql", + form: "complete", + origin: "regenerated", + }, + ], + }; + }, + ...overrides, + }); + } + + const expectation = { + containers: [["app"]], + kinds: { table: 2, view: 1, function: 2 }, + sampleObject: { path: ["app", "order_summary"], kind: "view" }, + absentSource: { path: ["app", absentName], kind: "function" }, + }; + + // The one positive case. It is here so the twelve refusals below are known to be + // refusing something a correct provider does not do. + test("passes a provider whose declaration, method and answers agree", async () => { + await assertObjectSurface(sourceProvider() as never, expectation); + }); + + // The pairing, in both directions. It is the only assertion in this block that certifies + // a provider implementing NOTHING, which is fifteen of the seventeen on the day it lands. + test("a provider that declares a source-bearing kind and implements no method is refused", async () => { + const provider = sourceProvider({ readObjectSource: undefined }); + await expect(assertObjectSurface(provider as never, expectation)).rejects.toThrow( + /declares 1 source-bearing kind\(s\) and does not implement readObjectSource/, + ); + }); + + test("a provider that implements the method and declares no source-bearing kind is refused", async () => { + const provider = sourceProvider({ getCapabilities: () => capabilities({ hasSource: undefined }) }); + await expect(assertObjectSurface(provider as never, expectation)).rejects.toThrow( + /declares 0 source-bearing kind\(s\) and implements readObjectSource/, + ); + }); + + test("an expectation with no absentSource is refused, so the absence raise is always driven", async () => { + const { absentSource: _absentSource, ...noAbsent } = expectation; + await expect(assertObjectSurface(sourceProvider() as never, noAbsent)).rejects.toThrow(/names no absentSource/); + }); + + // ONE notch narrower than "exercised none", which is where this class of guard has been + // wrong three times in this epic: Oracle declares nine source-bearing kinds, and an + // expectation naming one of them would silence a "none" guard while eight went unread. + test("an expectation that exercises only some source-bearing kinds is refused by name", async () => { + const twoSourceKinds = sourceProvider({ + getCapabilities: () => ({ + ...capabilities(), + objectKinds: capabilities().objectKinds.map((kind) => + kind.id === "view" ? { ...kind, hasSource: true, sourceLanguage: "sql" } : kind, + ), + }), + }); + const { view: _view, ...withoutView } = expectation.kinds; + await expect(assertObjectSurface(twoSourceKinds as never, { ...expectation, kinds: withoutView })).rejects.toThrow( + /source-bearing kinds the expectation never exercised \(view\)/, + ); + }); + + test("an absentSource naming a kind the source loop never read has no control, and is refused", async () => { + await expect( + assertObjectSurface(sourceProvider() as never, { + ...expectation, + absentSource: { path: ["app", "nope"], kind: "table" }, + }), + ).rejects.toThrow(/absentSource names the kind "table", which the source loop never read/); + }); + + // Both doubles below are correct in EVERY other respect: they raise for the absent path and + // they honour the caller's bound. Without that the helper throws for the bound instead and + // the assertion pins nothing, which a mutation of the path and kind checks proved. + test("a document answering for a different path than it was asked is refused", async () => { + const provider = sourceProvider({ + readObjectSource: async (path: readonly string[], kind: string, limit?: number) => { + raiseIfAbsent(path); + return { + path: ["app", "somewhere_else"], + kind, + parts: [ + { + id: "definition", + label: "Definition", + ...applySourceBound(readable, limit), + language: "sql", + form: "complete", + origin: "stored", + }, + ], + }; + }, + }); + await expect(assertObjectSurface(provider as never, expectation)).rejects.toThrow(/somewhere_else/); + }); + + test("a document answering for a different kind than it was asked is refused", async () => { + const provider = sourceProvider({ + readObjectSource: async (path: readonly string[], _kind: string, limit?: number) => { + raiseIfAbsent(path); + return { + path, + kind: "procedure", + parts: [ + { + id: "definition", + label: "Definition", + ...applySourceBound(readable, limit), + language: "sql", + form: "complete", + origin: "stored", + }, + ], + }; + }, + }); + await expect(assertObjectSurface(provider as never, expectation)).rejects.toThrow(/procedure/); + }); + + test("a part with an empty text is refused, because an empty definition is not a definition", async () => { + const provider = sourceProvider({ + readObjectSource: async (path: readonly string[], kind: string) => ({ + path, + kind, + parts: [ + { id: "definition", label: "Definition", text: " ", language: "sql", form: "complete", origin: "stored" }, + ], + }), + }); + await expect(assertObjectSurface(provider as never, expectation)).rejects.toThrow(/answered a part with no text/); + }); + + test("a refusal with an empty sentence is refused", async () => { + const provider = sourceProvider({ + readObjectSource: async (path: readonly string[], kind: string) => ({ + path, + kind, + parts: [{ id: "definition", label: "Definition", unavailable: " " }], + }), + }); + await expect(assertObjectSurface(provider as never, expectation)).rejects.toThrow( + /answered a refusal with no sentence/, + ); + }); + + test("two parts sharing one id are refused, because neither can be addressed", async () => { + const provider = sourceProvider({ + readObjectSource: async (path: readonly string[], kind: string) => ({ + path, + kind, + parts: [ + { + id: "definition", + label: "Specification", + text: readable, + language: "sql", + form: "complete", + origin: "stored", + }, + { id: "definition", label: "Body", text: readable, language: "sql", form: "complete", origin: "stored" }, + ], + }), + }); + await expect(assertObjectSurface(provider as never, expectation)).rejects.toThrow(/share the id "definition"/); + }); + + test("a language that disagrees with the kind's declared default is refused", async () => { + const provider = sourceProvider({ + getCapabilities: () => capabilities({ sourceLanguage: "lua" }), + }); + await expect(assertObjectSurface(provider as never, expectation)).rejects.toThrow( + /declares sourceLanguage "lua" and the part carries "sql"/, + ); + }); + + // A kind that declares no `sourceLanguage` leaves the part's id to the provider, so the + // comparison is skipped rather than defaulted. Without this the arm never runs. + test("a kind that declares no sourceLanguage accepts whatever the part carries", async () => { + const provider = sourceProvider({ getCapabilities: () => capabilities({ sourceLanguage: undefined }) }); + await assertObjectSurface(provider as never, expectation); + }); + + test("a provider whose every part is a refusal cannot be bound-checked, and says so", async () => { + const provider = sourceProvider({ + readObjectSource: async (path: readonly string[], kind: string) => ({ + path, + kind, + parts: [{ id: "definition", label: "Definition", unavailable: "Encrypted." }], + }), + }); + await expect(assertObjectSurface(provider as never, expectation)).rejects.toThrow( + /no source-bearing kind answered a readable part/, + ); + }); + + test("a provider whose longest definition is under the probe cannot be bound-checked, and says so", async () => { + const provider = sourceProvider({ + readObjectSource: async (path: readonly string[], kind: string) => ({ + path, + kind, + parts: [ + { id: "definition", label: "Definition", text: "x", language: "sql", form: "complete", origin: "stored" }, + ], + }), + }); + await expect(assertObjectSurface(provider as never, expectation)).rejects.toThrow( + /so a bound cannot be told from no bound/, + ); + }); + + test("a provider that ignores the limit is refused", async () => { + const provider = sourceProvider({ + readObjectSource: async (path: readonly string[], kind: string) => ({ + path, + kind, + parts: [ + { + id: "definition", + label: "Definition", + text: readable, + language: "sql", + form: "complete", + origin: "stored", + }, + ], + }), + }); + await expect(assertObjectSurface(provider as never, expectation)).rejects.toThrow( + /limit \d+\) returned \d+ characters/, + ); + }); + + test("a provider that bounds a part and reports nothing is refused", async () => { + const provider = sourceProvider({ + readObjectSource: async (path: readonly string[], kind: string, limit?: number) => ({ + path, + kind, + parts: [ + { + id: "definition", + label: "Definition", + text: limit === undefined ? readable : readable.slice(0, limit), + language: "sql", + form: "complete", + origin: "stored", + }, + ], + }), + }); + await expect(assertObjectSurface(provider as never, expectation)).rejects.toThrow(/reported no truncation/); + }); + + test("a provider that reports its bound in its own words, without the shared sentence, is refused", async () => { + const provider = sourceProvider({ + readObjectSource: async (path: readonly string[], kind: string, limit?: number) => ({ + path, + kind, + parts: [ + { + id: "definition", + label: "Definition", + text: limit === undefined ? readable : readable.slice(0, limit), + language: "sql", + form: "complete", + origin: "stored", + ...(limit === undefined ? {} : { truncated: { limit, reason: "source limit reached" } }), + }, + ], + }), + }); + await expect(assertObjectSurface(provider as never, expectation)).rejects.toThrow( + /which does not carry the one sentence a caller's bound is reported with/, + ); + }); + + // The NUMBER and the SENTENCE are two facts and a provider can get one right while the + // other is wrong. This double reports the correct sentence beside a limit it never applied, + // which is exactly what a reader would be shown as the size of the bound. + test("a provider that reports the right sentence beside the wrong limit number is refused", async () => { + const provider = sourceProvider({ + readObjectSource: async (path: readonly string[], kind: string, limit?: number) => { + raiseIfAbsent(path); + const bounded = applySourceBound(readable, limit); + return { + path, + kind, + parts: [ + { + id: "definition", + label: "Definition", + text: bounded.text, + language: "sql", + form: "complete", + origin: "stored", + ...(bounded.truncated === undefined + ? {} + : { truncated: { limit: bounded.truncated.limit + 1, reason: bounded.truncated.reason } }), + }, + ], + }; + }, + }); + await expect(assertObjectSurface(provider as never, expectation)).rejects.toThrow(); + }); + + // A provider with a SECOND bound of its own names both, so the guard asks for CONTAINS + // and never for equality. + test("accepts a bound reason that carries the shared sentence inside a composed one", async () => { + const provider = sourceProvider({ + readObjectSource: async (path: readonly string[], kind: string, limit?: number) => { + raiseIfAbsent(path); + const bounded = applySourceBound(readable, limit); + return { + path, + kind, + parts: [ + { + id: "definition", + label: "Definition", + text: bounded.text, + language: "sql", + form: "complete", + origin: "stored", + ...(bounded.truncated === undefined + ? {} + : { + truncated: { + limit: bounded.truncated.limit, + reason: `${bounded.truncated.reason}, and the catalog stores only the first 4,000 characters`, + }, + }), + }, + ], + }; + }, + }); + await assertObjectSurface(provider as never, expectation); + }); + + // The escape hatch a bounded probe would otherwise leave open: a provider could answer + // short on an unbounded call and wave the caller's flag at it. + test("a provider that reports a caller's bound on an UNBOUNDED read is refused", async () => { + const provider = sourceProvider({ + readObjectSource: async (path: readonly string[], kind: string) => ({ + path, + kind, + parts: [ + { + id: "definition", + label: "Definition", + text: readable, + language: "sql", + form: "complete", + origin: "stored", + truncated: { limit: 4, reason: sourceBoundTruncationReason(4) }, + }, + ], + }), + }); + await expect(assertObjectSurface(provider as never, expectation)).rejects.toThrow( + /was called with no limit and reported one/, + ); + }); + + // A bound of the provider's OWN on an unbounded read stays certifiable, which is the + // other side of the same assertion: redis and libredb walk a bounded keyspace and say so. + test("accepts a provider's own bound on an unbounded read", async () => { + const provider = sourceProvider({ + readObjectSource: async (path: readonly string[], kind: string, limit?: number) => { + raiseIfAbsent(path); + const bounded = applySourceBound(readable, limit); + return { + path, + kind, + parts: [ + { + id: "definition", + label: "Definition", + text: bounded.text, + language: "sql", + form: "complete", + origin: "stored", + truncated: bounded.truncated ?? { limit: 4000, reason: "the catalog stores only 4,000 characters" }, + }, + ], + }; + }, + }); + await assertObjectSurface(provider as never, expectation); + }); + + test("a provider that answers a document for an absent object instead of raising is refused", async () => { + const provider = sourceProvider({ + readObjectSource: async (path: readonly string[], kind: string, limit?: number) => ({ + path, + kind, + parts: [ + { + id: "definition", + label: "Definition", + ...applySourceBound(readable, limit), + language: "sql", + form: "complete", + origin: "stored", + }, + ], + }), + }); + await expect(assertObjectSurface(provider as never, expectation)).rejects.toThrow( + /did not raise for \["app","no_such_routine\(integer\)"\]; it answered a document/, + ); + }); + + test("a provider that raises something other than a QueryError for an absent object is refused", async () => { + const provider = sourceProvider({ + readObjectSource: async (path: readonly string[], kind: string, limit?: number) => { + if (path[path.length - 1] === "no_such_routine(integer)") throw new TypeError("undefined is not a function"); + return { + path, + kind, + parts: [ + { + id: "definition", + label: "Definition", + ...applySourceBound(readable, limit), + language: "sql", + form: "complete", + origin: "stored", + }, + ], + }; + }, + }); + await expect(assertObjectSurface(provider as never, expectation)).rejects.toThrow( + /did not raise for .*; it answered TypeError: undefined is not a function/, + ); + }); + + test("a raise that does not name the object is refused, because the reader cannot tell which one", async () => { + const provider = sourceProvider({ + readObjectSource: async (path: readonly string[], kind: string, limit?: number) => { + if (path[path.length - 1] === "no_such_routine(integer)") throw new QueryError("not found", "postgres"); + return { + path, + kind, + parts: [ + { + id: "definition", + label: "Definition", + ...applySourceBound(readable, limit), + language: "sql", + form: "complete", + origin: "stored", + }, + ], + }; + }, + }); + await expect(assertObjectSurface(provider as never, expectation)).rejects.toThrow( + /without naming "no_such_routine\(integer\)"/, + ); + }); +}); From aa8adf0ac1d1d75ee9f9236614cf9687169dc567 Mon Sep 17 00:00:00 2001 From: cevheri Date: Sat, 12 Sep 2026 21:13:11 +0300 Subject: [PATCH 03/94] fix(objects): refuse a part that is both a refusal and a text, and raise a dropped Redis socket (#789) Fix round 1 on the object source contract. The union does NOT make a part carrying both `text` and `unavailable` unrepresentable: measured against tsc 6.0.3 with no cast, such a literal compiles, because TypeScript's excess-property check on a union admits any property declared on any member. `isSourcePartUnavailable` then narrows it to the refusal arm and the document walk continued past every remaining check, so a Source pane would render a refusal sentence over a definition the engine really returned. `assertSourceDocument` now throws for it before the narrowing, and both docblocks that claimed the type closed this say what was measured. The Redis refusal arm caught every rejection. Measured against ioredis 5.11.1 and redis 8.10.0: a server error reply is a `ReplyError`, a dropped socket is a plain Error reading "Connection is closed.", and the old catch presented the second as the server's own refusal of this object. Only a server error reply is a refusal now; everything else raises a ConnectionError naming the library. `applySourceBound` cut UTF-16 code units, so a bound landing between a surrogate pair emitted a lone surrogate as the last character of a part. It drops the unpaired half and still reports the caller's number as the bound. Three test repairs: the `readonly` predicate test narrowed a const at its declaration and so passed for any predicate spelling, the wrong-limit-number test threw with no pattern, and the two unmarked `applySourceBound` cases used `toEqual`, which ignores an explicitly-undefined property. Each is now killed by the mutation it exists for. --- docs/providers/redis.md | 11 ++++ src/lib/db/object-kinds.ts | 14 ++++- src/lib/db/providers/keyvalue/redis.ts | 39 +++++++++++- src/lib/db/types.ts | 14 ++++- tests/helpers/object-surface-conformance.ts | 45 +++++++++++-- tests/integration/db/redis-provider.test.ts | 58 ++++++++++++++++- tests/unit/db/object-kinds.test.ts | 63 +++++++++++++++++-- .../db/object-surface-conformance.test.ts | 41 +++++++++++- 8 files changed, 267 insertions(+), 18 deletions(-) diff --git a/docs/providers/redis.md b/docs/providers/redis.md index d310f6b6..82f071ac 100644 --- a/docs/providers/redis.md +++ b/docs/providers/redis.md @@ -898,8 +898,19 @@ KeyDB, DragonflyDB and Garnet have no `FUNCTION` command at all and each refuses table in [§6.1](#61-the-object-surface-789) has them), so this path is reachable on three of the four Redis-wire relatives this type id serves. +**Only the server's own error reply is a refusal. A transport failure RAISES.** +Measured against ioredis 5.11.1 and Redis 8.10.0: an ACL denial and an unknown command both reject with +a `redis-errors` `ReplyError`, whose `name` is `ReplyError`, while a dropped socket rejects with a plain +`Error` named `Error`, reading `Connection is closed.` with the offline queue on and +`Stream isn't writeable and enableOfflineQueue options is false` with it off. +A read that catches both would show `Connection is closed.` in the Source pane as this object's own +refusal, with no raise, no retry affordance and nothing in the document telling it apart from a real +`NOPERM`, so the provider raises a `ConnectionError` naming the library instead. + **A caller's bound** cuts one part's text and reports itself through the one sentence every engine uses, `the source read was bounded at characters by its caller`. An exact answer is never marked. +The bound counts UTF-16 code units, so a bound landing between the two halves of a surrogate pair drops +the pair rather than emitting a lone surrogate; the mark still names the number the caller asked for. #### Reads go to the CONTAINER's database, never the session's diff --git a/src/lib/db/object-kinds.ts b/src/lib/db/object-kinds.ts index 30401a4d..fd3c9032 100644 --- a/src/lib/db/object-kinds.ts +++ b/src/lib/db/object-kinds.ts @@ -197,5 +197,17 @@ export function applySourceBound( limit: number | undefined, ): { readonly text: string; readonly truncated?: { readonly limit: number; readonly reason: string } } { if (limit === undefined || text.length <= limit) return { text }; - return { text: text.slice(0, limit), truncated: { limit, reason: sourceBoundTruncationReason(limit) } }; + const cut = text.slice(0, limit); + // The bound counts UTF-16 CODE UNITS, so it can land BETWEEN the two halves of a surrogate + // pair, and an astral character is exactly that: a PL/pgSQL body or a Lua library holding an + // emoji, cut at that offset, would end in an unpaired high surrogate. That is not a + // character, JSON serializes it as a lone escape and Monaco draws a replacement glyph, so + // the pair is dropped whole. The last unit of the cut can only BE a high surrogate when its + // low half sits at `limit` in the original, because this arm runs only when the text is + // longer than the bound. `truncated.limit` still names the CALLER's number rather than the + // emitted length: the bound is what was asked for, and reporting anything else describes a + // bound nobody set. + const last = cut.charCodeAt(cut.length - 1); + const kept = last >= 0xd800 && last <= 0xdbff ? cut.slice(0, -1) : cut; + return { text: kept, truncated: { limit, reason: sourceBoundTruncationReason(limit) } }; } diff --git a/src/lib/db/providers/keyvalue/redis.ts b/src/lib/db/providers/keyvalue/redis.ts index 20c3b3d4..7ccc309e 100644 --- a/src/lib/db/providers/keyvalue/redis.ts +++ b/src/lib/db/providers/keyvalue/redis.ts @@ -350,7 +350,6 @@ function parseFunctionLibraries(reply: unknown): string[] { * One library's `library_code` out of a `FUNCTION LIST ... WITHCODE` reply, selected * BYTE-EQUAL (#789 Phase 2). * - * The selection is the whole of this function's reason to exist. MEASURED on redis 8.10.0 * against the committed fixture: the library dictionary is CASE-SENSITIVE, so `libredb_probe` * and `LIBREDB_PROBE` coexist, while the `LIBRARYNAME` argument is a CASE-INSENSITIVE glob, so @@ -363,6 +362,27 @@ function parseFunctionLibraries(reply: unknown): string[] { * the nested `functions` value is itself a list of key/value lists, so a parser reading * positions takes a field name for a library name the moment the server adds a field. */ +/** + * Whether a driver rejection is the SERVER's own error reply, rather than a transport + * failure (#789 Phase 2). + * + * MEASURED against ioredis 5.11.1 and redis 8.10.0, from a container created for the + * measurement: an ACL denial rejects with a `redis-errors` `ReplyError` + * (`constructor.name` and `name` both "ReplyError") carrying "NOPERM User ... has no + * permissions to run the 'function|list' command", and so does an unknown command + * ("ERR unknown command 'NOSUCHCOMMAND'"). A DROPPED SOCKET rejects with a plain `Error` + * named "Error", message "Connection is closed." with the offline queue on and "Stream + * isn't writeable and enableOfflineQueue options is false" with it off. + * + * The NAME and not `instanceof`: ioredis re-exports the class, but the integration suite + * replaces the whole module with `mock.module`, so an `instanceof` against the driver's + * export would be `instanceof undefined` there. `redis-errors` sets `name` on the + * prototype, so the name is the one fact both the real driver and a double can carry. + */ +function isServerErrorReply(error: unknown): boolean { + return error instanceof Error && error.name === "ReplyError"; +} + function parseFunctionLibraryCode(reply: unknown, name: string): string | undefined { for (const entry of Array.isArray(reply) ? reply : []) { if (!Array.isArray(entry)) continue; @@ -1242,6 +1262,11 @@ export class RedisProvider extends BaseDatabaseProvider { * `FUNCTION` command at all and each refuses in its own words (all measured 2026-09-11), so * this path is reachable on three of the four Redis-wire relatives this type id serves. * + * A refusal is ONLY the server's own error reply. A TRANSPORT failure RAISES, because it is + * nobody answering rather than the server answering "no", and a pane reading "Connection is + * closed." as this object's refusal would be a symptom presented as a fact about the + * object. `isServerErrorReply` carries the measurement that tells the two apart. + * * The name is `path[path.length - 1]` and never `path[1]`: standing ruling 5g, and the * integration suite pins it by swapping a two-level declaration in. */ @@ -1257,6 +1282,18 @@ export class RedisProvider extends BaseDatabaseProvider { try { reply = await this.callFunctionList(name); } catch (error) { + // ONLY the server's own error reply is a refusal. A transport failure is nobody + // answering at all, and answering a document for it would put "Connection is closed." + // in the Source pane as this object's own refusal, with no raise, no retry affordance + // and nothing in the document telling it apart from a real NOPERM. The two shapes are + // measured on `isServerErrorReply`. + if (!isServerErrorReply(error)) { + throw new ConnectionError( + `Failed to read the Redis function library ${JSON.stringify(name)}: ` + + `${error instanceof Error ? error.message : String(error)}`, + "redis", + ); + } return { path: [...path], kind, diff --git a/src/lib/db/types.ts b/src/lib/db/types.ts index e2e76407..e71afc57 100644 --- a/src/lib/db/types.ts +++ b/src/lib/db/types.ts @@ -1380,11 +1380,21 @@ export type ObjectSourceOrigin = "stored" | "regenerated" | "rendered"; * * A UNION and not one shape with an optional `text`, for the reason `KindCount` is a union: a * refusal and an empty answer are different facts, and a shape carrying `text?: string` makes - * them the same value at every call site. The refused arm has NO `text` key at all, so there is - * no path from a refusal to an editor buffer. That composition is what DBeaver gets wrong: + * them the same value at every call site. The refused arm declares NO `text`, so a value + * narrowed to it cannot reach an editor buffer. That composition is what DBeaver gets wrong: * measured in its source, an unreadable definition reaches a WRITABLE editor holding one * comment line. * + * The union closes that path in ONE DIRECTION ONLY, and saying so here is what stops the next + * implementer from trusting it for the other. MEASURED against tsc 6.0.3 with no cast + * anywhere: a literal carrying `unavailable` BESIDE `text`, `language`, `form` and `origin` + * COMPILES as an `ObjectSourcePart`, because TypeScript's excess-property check on a union + * admits any property declared on ANY member of it. Such a part narrows to the refusal arm, so + * a provider composing one (spreading a catalog row, or spreading a conditional + * `{unavailable}` onto a bounded text) would put a refusal sentence over a definition the + * engine really returned. `assertObjectSurface` refuses that part by name for our own + * providers, and the client's shape check refuses it for a host's answer. + * * `id` is provider-local. Core reads it as an identity WITHIN ONE DOCUMENT and for nothing * else: the part switcher's selection key, and the Source tab's remembered selection. Core * never compares it against a literal, never branches on it, and never carries it between two diff --git a/tests/helpers/object-surface-conformance.ts b/tests/helpers/object-surface-conformance.ts index 93875c8e..6bf8057d 100644 --- a/tests/helpers/object-surface-conformance.ts +++ b/tests/helpers/object-surface-conformance.ts @@ -451,8 +451,17 @@ async function assertSourceSurface( // NAMED rather than counted above zero. A kind the expectation names at zero is an // acknowledged absence that the count assertion already pins, and a fixture holding none of - // a declared kind is a legitimate state ruling 4 describes. A kind the expectation OMITS is - // the silent hole, and it is the one refused here. + // a declared kind is a legitimate state. A kind the expectation OMITS is the silent hole, + // and it is the one refused here. + // + // WHAT THIS LEAVES OPEN, stated so no provider task has to discover it: a source-bearing + // kind named at ZERO has its `readObjectSource` path driven by NOTHING here, because there + // is no object of it to read. Only the OMITTED case is refused. On an engine declaring many + // source-bearing kinds, a fixture holding none of one of them therefore drops that kind's + // read out of the contract entirely while every assertion stays green. The remedy is the + // FIXTURE and not this helper: build one that holds an object of every source-bearing kind + // the engine declares. The `longest === undefined` throw below bounds the damage by + // requiring at least one readable part from at least one kind, and that is all it does. const unexercised = sourceKinds.filter((kind) => !Object.hasOwn(expected.kinds, kind.id)).map((kind) => kind.id); if (unexercised.length > 0) { throw new Error( @@ -512,7 +521,17 @@ async function assertSourceSurface( throw new Error(`readObjectSource("${longest.kind}", limit ${probe}) returned ${part.text.length} characters`); } if (part.truncated === undefined) continue; - expect(part.truncated.limit).toBe(probe); + // The NUMBER and the SENTENCE are two facts and a provider can get one right while the + // other is wrong. An explicit throw rather than a bare `expect`, so the test that drives + // this can pin the MESSAGE: `.rejects.toThrow()` with no pattern passes for any throw + // ahead of it, which is how two doubles in this file were vacuous before. + if (part.truncated.limit !== probe) { + throw new Error( + `readObjectSource("${longest.kind}", limit ${probe}) reported the bound as ${part.truncated.limit}, ` + + `which is not the limit it was given (${probe}), and that number is what a reader is shown as ` + + "the size of the bound", + ); + } const sentence = sourceBoundTruncationReason(probe); if (!part.truncated.reason.includes(sentence)) { throw new Error( @@ -560,9 +579,16 @@ async function assertSourceSurface( /** * One document's own shape. * - * What is NOT checked here, and why: a part carrying BOTH `text` and `unavailable` is a - * compile error for our own providers, so a check for it here would be a line no test can - * reach. A HOST can produce one, and the client's shape check is where that is caught. + * A part carrying BOTH `text` and `unavailable` IS checked here, and the reason is a + * correction of what this docblock claimed first. The claim was that the union makes the + * shape a compile error for our own providers. MEASURED against tsc 6.0.3, with NO cast + * anywhere: `{ id, label, text, language, form, origin, unavailable }` compiles as an + * `ObjectSourcePart`, because TypeScript's excess-property check on a UNION admits any + * property declared on ANY member of it, so the refusal key is legal on the readable arm. + * `isSourcePartUnavailable` then narrows it to the refusal arm and this walk would continue + * past every check below, certifying a well-formed refusal over a definition the engine + * really returned. The type closes the path from a refusal to an editor buffer in ONE + * direction only, and this throw closes the other. */ function assertSourceDocument( document: ObjectSourceDocument, @@ -578,6 +604,13 @@ function assertSourceDocument( throw new Error(`two parts of ${JSON.stringify(object.path)} share the id "${part.id}"`); } ids.add(part.id); + // BEFORE the narrowing, because the refusal branch continues past every check below it. + if ("unavailable" in part && "text" in part) { + throw new Error( + `readObjectSource("${kindId}") answered a part that carries both a refusal and a text; ` + + "a refusal and a definition are different facts and a reader must never be shown one over the other", + ); + } if (isSourcePartUnavailable(part)) { if (part.unavailable.trim() === "") { throw new Error(`readObjectSource("${kindId}") answered a refusal with no sentence a person can read`); diff --git a/tests/integration/db/redis-provider.test.ts b/tests/integration/db/redis-provider.test.ts index 66b34c42..9f1f4bb3 100644 --- a/tests/integration/db/redis-provider.test.ts +++ b/tests/integration/db/redis-provider.test.ts @@ -231,6 +231,32 @@ let functionWithCodeReply: unknown[] = MOCK_FUNCTION_WITHCODE; */ let functionRefusal: string | null = null; +/** + * When set, the `FUNCTION` command rejects the way a TRANSPORT failure does rather than the + * way a server refusal does. + * + * MEASURED against ioredis 5.11.1 and redis 8.10.0 from a container created for this + * measurement: a server ERROR REPLY arrives as a `redis-errors` `ReplyError` + * (`name === "ReplyError"`), while a dropped socket arrives as a PLAIN `Error` named `Error`, + * message "Connection is closed." with the offline queue on and "Stream isn't writeable and + * enableOfflineQueue options is false" with it off. The two are different facts and the + * provider must not present the second as the server refusing this object (#789). + */ +let functionTransportFailure: unknown = null; + +/** + * A server error reply, shaped as ioredis 5.11.1 delivers one. + * + * `redis-errors` sets `name` to "ReplyError" on the prototype and ioredis re-exports the + * class, so the name is what the provider reads: an `instanceof` against the driver's export + * would be `instanceof undefined` here, where `mock.module` replaces the whole module. + */ +function replyError(message: string): Error { + const error = new Error(message); + error.name = "ReplyError"; + return error; +} + /** When set, `SCAN` rejects with this sentence, whatever database it was opened on. */ let scanRefusal: string | null = null; @@ -339,7 +365,8 @@ mock.module("ioredis", () => { } if (cmd === "CONFIG") return databasesReply; if (cmd === "FUNCTION") { - if (functionRefusal !== null) throw new Error(functionRefusal); + if (functionTransportFailure !== null) throw functionTransportFailure; + if (functionRefusal !== null) throw replyError(functionRefusal); // WITHCODE is the source read and LIST without it is the listing. The two answer // different shapes on a real server and the mock has to as well, or a provider // reading `library_code` off the listing reply would pass. @@ -1317,6 +1344,7 @@ describe("RedisProvider", () => { functionListReply = MOCK_FUNCTION_LIST; functionWithCodeReply = MOCK_FUNCTION_WITHCODE; functionRefusal = null; + functionTransportFailure = null; scanRefusal = null; scanOverflows = false; scanCalls = 0; @@ -1421,6 +1449,34 @@ describe("RedisProvider", () => { expect("text" in part).toBe(false); }); + /* + A REFUSAL is the server answering "no". A TRANSPORT failure is nobody answering at all, + and the two must not arrive at the same pane. MEASURED against ioredis 5.11.1 and redis + 8.10.0: a dropped socket rejects with a PLAIN Error reading "Connection is closed.", + while an ACL denial rejects with a `ReplyError`. A bare `catch` around the command turns + the first into a Source pane reading "Connection is closed." presented as this object's + own refusal, with no raise, no retry affordance and nothing in the document telling it + apart from a real NOPERM. It raises instead, and the fifteen providers copying this arm + copy the distinction with it. + */ + test("a dropped connection RAISES rather than being presented as the server's refusal", async () => { + functionTransportFailure = new Error("Connection is closed."); + + await expect(provider.readObjectSource!(["0", "libredb_probe"], "function")).rejects.toThrow( + /Failed to read the Redis function library "libredb_probe": Connection is closed\./, + ); + }); + + test("a rejection that is not an Error at all raises too, rather than becoming a refusal sentence", async () => { + // A driver is free to reject with something that is not an `Error`, and the class check + // must not read that as a server reply by omission. + functionTransportFailure = "socket hang up"; + + await expect(provider.readObjectSource!(["0", "libredb_probe"], "function")).rejects.toThrow( + /Failed to read the Redis function library "libredb_probe": socket hang up/, + ); + }); + test("a library whose code the server withheld is absence rather than an empty definition", async () => { // The entry matches by name and carries no `library_code`. Answering a part with an // empty text would put an empty editor over a definition that was never read, which is diff --git a/tests/unit/db/object-kinds.test.ts b/tests/unit/db/object-kinds.test.ts index b84a8151..3088819a 100644 --- a/tests/unit/db/object-kinds.test.ts +++ b/tests/unit/db/object-kinds.test.ts @@ -182,20 +182,39 @@ describe("isSourcePartUnavailable", () => { test("narrows a readable part in the other direction, so the caller reaches text", () => { expect(isSourcePartUnavailable(readable)).toBe(false); - if (isSourcePartUnavailable(readable)) throw new Error("unreachable"); - // This line is the whole point of the `readonly` spelling: without it the false branch - // still holds the union and `.text` does not exist. - expect(readable.text).toBe("SELECT 1"); + // The values walked here are typed as the WHOLE union and are NOT narrowed by their own + // initializers, which is the only shape in which the predicate's FALSE branch is + // load-bearing. MEASURED: with `readonly` dropped from the three members of the + // predicate's return type, `part.text` below is TS2339 and `bun run typecheck` fails on + // this file. The earlier spelling of this test narrowed a `const` at its declaration, so + // `.text` resolved whether the predicate narrowed the false branch or not, and the same + // mutation left this file with zero errors. + const parts: readonly ObjectSourcePart[] = [readable, refused]; + const texts: string[] = []; + for (const part of parts) { + if (isSourcePartUnavailable(part)) continue; + texts.push(part.text); + } + expect(texts).toEqual(["SELECT 1"]); }); }); describe("applySourceBound", () => { test("an unbounded call marks nothing, because marking an exact answer teaches a reader to discount every mark", () => { - expect(applySourceBound("SELECT 1", undefined)).toEqual({ text: "SELECT 1" }); + const bounded = applySourceBound("SELECT 1", undefined); + + expect(bounded).toEqual({ text: "SELECT 1" }); + // `toEqual` IGNORES an explicitly-undefined property, so the line above passes for an + // implementation answering `{ text, truncated: undefined }`. The part shape and the + // source route both read the KEY's ABSENCE, so the key is what is asserted. + expect(Object.hasOwn(bounded, "truncated")).toBe(false); }); test("a text that fits its bound is not marked either", () => { - expect(applySourceBound("SELECT 1", 8)).toEqual({ text: "SELECT 1" }); + const bounded = applySourceBound("SELECT 1", 8); + + expect(bounded).toEqual({ text: "SELECT 1" }); + expect(Object.hasOwn(bounded, "truncated")).toBe(false); }); test("a text over its bound is sliced and marked with the one sentence", () => { @@ -203,6 +222,38 @@ describe("applySourceBound", () => { expect(bounded.text).toBe("SELECT"); expect(bounded.truncated).toEqual({ limit: 6, reason: sourceBoundTruncationReason(6) }); }); + + /* + A bound cuts UTF-16 CODE UNITS, and an astral character is two of them. A PL/pgSQL body + or a Lua library holding an emoji or an astral CJK character, bounded at exactly the + offset between the pair, would otherwise end in an unpaired high surrogate: JSON + serializes it as a lone \ud83d and Monaco renders a replacement glyph. Redis cannot + reach this through its own fixture, and every one of the remaining sixteen providers + routes its text through this one function, which is why the guard lives here. + */ + test("a bound landing inside a surrogate pair cuts before it, never emitting a lone surrogate", () => { + const bounded = applySourceBound("a\u{1F600}b", 2); + + expect(bounded.text).toBe("a"); + expect([...bounded.text]).toHaveLength(1); + // The mark still names the CALLER's number. It is the bound that was asked for, and a + // provider reporting the emitted length instead would tell a reader a bound it never set. + expect(bounded.truncated).toEqual({ limit: 2, reason: sourceBoundTruncationReason(2) }); + }); + + test("a bound landing after a whole surrogate pair keeps the pair", () => { + const bounded = applySourceBound("a\u{1F600}b", 3); + + expect(bounded.text).toBe("a\u{1F600}"); + expect([...bounded.text]).toHaveLength(2); + }); + + test("a bound of zero answers an empty text rather than reading past the start", () => { + const bounded = applySourceBound("SELECT 1", 0); + + expect(bounded.text).toBe(""); + expect(bounded.truncated).toEqual({ limit: 0, reason: sourceBoundTruncationReason(0) }); + }); }); describe("the source bounds", () => { diff --git a/tests/unit/db/object-surface-conformance.test.ts b/tests/unit/db/object-surface-conformance.test.ts index 16de2bca..9a7fd8e8 100644 --- a/tests/unit/db/object-surface-conformance.test.ts +++ b/tests/unit/db/object-surface-conformance.test.ts @@ -1223,7 +1223,46 @@ describe("assertObjectSurface and the object source read", () => { }; }, }); - await expect(assertObjectSurface(provider as never, expectation)).rejects.toThrow(); + await expect(assertObjectSurface(provider as never, expectation)).rejects.toThrow( + /reported the bound as 11, which is not the limit it was given \(10\)/, + ); + }); + + /* + The union does NOT make this shape unrepresentable, which is the opposite of what the + helper's own docblock claimed until this test was written. MEASURED with tsc 6.0.3 and + NO cast anywhere: a part literal carrying `unavailable` beside `text`, `language`, `form` + and `origin` compiles as an `ObjectSourcePart`, because TypeScript's excess-property check + on a union admits any property declared on ANY member of it. `isSourcePartUnavailable` + then narrows it to the refusal arm and the document-shape walk continues past every check + below, so a Source pane would render the refusal sentence over a definition the engine + really returned. That is the DBeaver shape this contract exists to make impossible, + running in the one direction the type does not close. + */ + test("a part carrying both a refusal and a text is refused, because the union does not stop it", async () => { + const provider = sourceProvider({ + readObjectSource: async (path: readonly string[], kind: string) => { + raiseIfAbsent(path); + return { + path, + kind, + parts: [ + { + id: "definition", + label: "Definition", + text: readable, + language: "sql", + form: "complete", + origin: "regenerated", + unavailable: "Encrypted.", + }, + ], + }; + }, + }); + await expect(assertObjectSurface(provider as never, expectation)).rejects.toThrow( + /answered a part that carries both a refusal and a text/, + ); }); // A provider with a SECOND bound of its own names both, so the guard asks for CONTAINS From bab3f87498f8d859df34537c5ab30cc79048e45c Mon Sep 17 00:00:00 2001 From: cevheri Date: Sat, 12 Sep 2026 21:24:50 +0300 Subject: [PATCH 04/94] fix(objects): reattach the Redis parser docblock and stop claiming a guard that is not written (#789) The round-1 repair inserted isServerErrorReply between parseFunctionLibraryCode's docblock and parseFunctionLibraryCode, so the function that selects a library by byte-equal name was undocumented and fifteen lines of measured Redis behaviour were attributed to the error classifier. Neither Biome nor oxlint nor ESLint reads comment adjacency, so a guard replaces the review note: two tests parse the provider with the TypeScript compiler API and assert that no module-private function is left undocumented and that the byte-equal rule is attached to the function that implements it. ObjectSourcePart's docblock said the client's shape check refuses a part carrying both text and unavailable, and refuses an empty text, for a host's answer. Nothing in this tree does either: the embedded seam's shape check is later work. Both sentences now say what stands today, and the forward reference points at isRenderableShape, which exists, rather than at a name no file defines. --- src/lib/db/providers/keyvalue/redis.ts | 32 ++++++------ src/lib/db/types.ts | 11 ++-- tests/integration/db/redis-provider.test.ts | 58 +++++++++++++++++++++ 3 files changed, 81 insertions(+), 20 deletions(-) diff --git a/src/lib/db/providers/keyvalue/redis.ts b/src/lib/db/providers/keyvalue/redis.ts index 7ccc309e..bf5587fb 100644 --- a/src/lib/db/providers/keyvalue/redis.ts +++ b/src/lib/db/providers/keyvalue/redis.ts @@ -362,6 +362,22 @@ function parseFunctionLibraries(reply: unknown): string[] { * the nested `functions` value is itself a list of key/value lists, so a parser reading * positions takes a field name for a library name the moment the server adds a field. */ +function parseFunctionLibraryCode(reply: unknown, name: string): string | undefined { + for (const entry of Array.isArray(reply) ? reply : []) { + if (!Array.isArray(entry)) continue; + let matched = false; + let code: string | undefined; + for (let index = 0; index + 1 < entry.length; index += 2) { + const key = String(entry[index]); + const value = entry[index + 1]; + if (key === "library_name" && value === name) matched = true; + if (key === "library_code" && typeof value === "string") code = value; + } + if (matched) return code; + } + return undefined; +} + /** * Whether a driver rejection is the SERVER's own error reply, rather than a transport * failure (#789 Phase 2). @@ -383,22 +399,6 @@ function isServerErrorReply(error: unknown): boolean { return error instanceof Error && error.name === "ReplyError"; } -function parseFunctionLibraryCode(reply: unknown, name: string): string | undefined { - for (const entry of Array.isArray(reply) ? reply : []) { - if (!Array.isArray(entry)) continue; - let matched = false; - let code: string | undefined; - for (let index = 0; index + 1 < entry.length; index += 2) { - const key = String(entry[index]); - const value = entry[index + 1]; - if (key === "library_name" && value === name) matched = true; - if (key === "library_code" && typeof value === "string") code = value; - } - if (matched) return code; - } - return undefined; -} - // ============================================================================ // Redis Provider // ============================================================================ diff --git a/src/lib/db/types.ts b/src/lib/db/types.ts index e71afc57..a67db058 100644 --- a/src/lib/db/types.ts +++ b/src/lib/db/types.ts @@ -1393,7 +1393,10 @@ export type ObjectSourceOrigin = "stored" | "regenerated" | "rendered"; * a provider composing one (spreading a catalog row, or spreading a conditional * `{unavailable}` onto a bounded text) would put a refusal sentence over a definition the * engine really returned. `assertObjectSurface` refuses that part by name for our own - * providers, and the client's shape check refuses it for a host's answer. + * providers, and that is the ONLY refusal standing today. A HOST's answer is unguarded: the + * embedded seam's runtime shape check, the one `isRenderableShape` in + * `src/components/object-tree/use-tree-nodes.ts` is the precedent for, is later work in #789 + * Phase 2 and does not exist in this tree. * * `id` is provider-local. Core reads it as an identity WITHIN ONE DOCUMENT and for nothing * else: the part switcher's selection key, and the Source tab's remembered selection. Core @@ -1401,9 +1404,9 @@ export type ObjectSourceOrigin = "stored" | "regenerated" | "rendered"; * documents. * * `text` is never empty and never whitespace only. TypeScript cannot express that, so it is a - * runtime invariant asserted in `assertObjectSurface` for our own providers and in the client's - * shape check for a host's answer. Where an engine answers empty, the provider emits a REFUSAL - * carrying the engine's own fact instead. + * runtime invariant, asserted in `assertObjectSurface` for our own providers and, for a host's + * answer, by the same shape check that does not exist yet. Where an engine answers empty, the + * provider emits a REFUSAL carrying the engine's own fact instead. */ export type ObjectSourcePart = | { diff --git a/tests/integration/db/redis-provider.test.ts b/tests/integration/db/redis-provider.test.ts index 9f1f4bb3..a1f9508a 100644 --- a/tests/integration/db/redis-provider.test.ts +++ b/tests/integration/db/redis-provider.test.ts @@ -5,6 +5,9 @@ * before importing the RedisProvider class. */ import { describe, test, expect, beforeEach, afterEach, mock, spyOn } from "bun:test"; +import { readFileSync } from "node:fs"; +import { join } from "node:path"; +import ts from "typescript"; import { assertObjectSurface } from "../../helpers/object-surface-conformance"; import { isSourcePartUnavailable, sourceBoundTruncationReason } from "@/lib/db/object-kinds"; import type { DatabaseConnection } from "@/lib/types"; @@ -2025,3 +2028,58 @@ describe("RedisProvider", () => { }); }); }); + +/** + * Every module-private function in this provider carries its OWN doc comment (#789). + * + * A guard and not a review note, because the defect it catches is invisible to both linters + * this repository runs. A new function inserted BETWEEN an existing docblock and the function + * that block documents leaves the original function undocumented and silently re-attributes + * the measurement to a different function. Neither Biome nor oxlint nor ESLint reads comment + * adjacency at all, and it happened here: `isServerErrorReply` landed between + * `parseFunctionLibraryCode`'s block and `parseFunctionLibraryCode`. + * + * Measured with the TypeScript compiler API rather than by reading the text, because the + * compiler resolves a block to the declaration it ACTUALLY attaches to, which is the whole + * question. `ts.getJSDocCommentsAndTags` over the real file on disk, the same mechanism the + * seven provider seam guards under `tests/unit/db` use. + * + * The blocks in this file are the only record of why the library selection is byte-equal and + * why a reply's pairs are walked rather than indexed, and this is the reference implementation + * fifteen provider tasks copy. + */ +describe("the Redis provider's own doc comments", () => { + const FILE = join(import.meta.dir, "..", "..", "..", "src", "lib", "db", "providers", "keyvalue", "redis.ts"); + const source = ts.createSourceFile(FILE, readFileSync(FILE, "utf8"), ts.ScriptTarget.Latest, true); + const functions = source.statements.filter(ts.isFunctionDeclaration); + const blocksOf = (declaration: ts.FunctionDeclaration) => ts.getJSDocCommentsAndTags(declaration).filter(ts.isJSDoc); + + test("no module-private function is left undocumented by a block that moved on to another", () => { + // Non-vacuity first: a guard over an empty enumeration passes forever, and this one reads + // a file it does not own the shape of. + const names = functions.map((declaration) => declaration.name?.text); + expect(names).toContain("parseFunctionLibraryCode"); + expect(names).toContain("isServerErrorReply"); + expect(functions.length).toBeGreaterThan(5); + + const undocumented = functions + .filter((declaration) => blocksOf(declaration).length === 0) + .map((declaration) => declaration.name?.text ?? ""); + expect(undocumented).toEqual([]); + }); + + test("the byte-equal selection rule is attached to the function that implements it", () => { + // The block names a rule about ONE function's behaviour, so it is worth nothing on any + // other: a reader asking why `reply[0]` is wrong for a WITHCODE reply finds it here or + // nowhere. + const declaration = functions.find((entry) => entry.name?.text === "parseFunctionLibraryCode"); + if (declaration === undefined) throw new Error("parseFunctionLibraryCode is gone from the provider"); + + const documented = blocksOf(declaration) + .map((block) => block.getText(source)) + .join("\n"); + + expect(documented).toContain("BYTE-EQUAL"); + expect(documented).toContain("01-object-fixture.redis"); + }); +}); From 48991f0d2cb8764b34216a0f96adeb40e7d4952a Mon Sep 17 00:00:00 2001 From: cevheri Date: Sat, 12 Sep 2026 21:42:46 +0300 Subject: [PATCH 05/94] refactor(editor): give the two Monaco themes one owner (#789) `db-dark` and `db-light` were defined inside `QueryEditor`'s `beforeMount`, which is per-mount state: a second Monaco mount that does not run that callback paints with Monaco's stock theme and sits visibly beside a query editor it does not match. The read-only object source viewer is that second mount. Both definitions move verbatim into `src/lib/editor/monaco-theme.ts`, which exports `defineStudioThemes` plus the two ids as constants so no mount can spell one wrong. The payloads are byte-identical to the ones that shipped, and the unit test pins both in full: a hoist that changes one hex digit is a visible regression nothing else here would catch. The module-scope `configureMonacoLoader()` call stays in `QueryEditor`, and `tests/isolated/monaco-loader-wiring.test.ts` still observes it at module-evaluation time. --- src/components/QueryEditor.tsx | 68 ++---------- src/lib/editor/monaco-theme.ts | 91 ++++++++++++++++ tests/unit/editor/monaco-theme.test.ts | 137 +++++++++++++++++++++++++ 3 files changed, 235 insertions(+), 61 deletions(-) create mode 100644 src/lib/editor/monaco-theme.ts create mode 100644 tests/unit/editor/monaco-theme.test.ts diff --git a/src/components/QueryEditor.tsx b/src/components/QueryEditor.tsx index 4cf26093..525b607a 100644 --- a/src/components/QueryEditor.tsx +++ b/src/components/QueryEditor.tsx @@ -13,6 +13,7 @@ import { registerMongoDBCompletionProvider } from "@/lib/editor/mongodb-completi import { registerLibreDBLanguage } from "@/lib/editor/libredb-language"; import { registerRedisLanguage } from "@/lib/editor/redis-language"; import { configureMonacoLoader } from "@/lib/editor/monaco-loader"; +import { defineStudioThemes, STUDIO_THEME_DARK, STUDIO_THEME_LIGHT } from "@/lib/editor/monaco-theme"; import { useEffectiveTheme } from "@/hooks/use-effective-theme"; import { useMonacoInstance } from "@/hooks/use-monaco-instance"; import { logger } from "@/lib/logger"; @@ -116,9 +117,10 @@ export const QueryEditor = forwardRef( const editorRef = useRef(null); const [hasSelection, setHasSelection] = useState(false); - // Both themes are defined in `beforeMount`; this only picks which is applied. + // Both themes are defined in `beforeMount`, from `@/lib/editor/monaco-theme`; this only picks + // which is applied. // Monaco re-reads the `theme` prop on change, so the switch needs no remount. - const editorTheme = useEffectiveTheme() === "light" ? "db-light" : "db-dark"; + const editorTheme = useEffectiveTheme() === "light" ? STUDIO_THEME_LIGHT : STUDIO_THEME_DARK; // Explain capability gate, shared by the toolbar button and the context-menu action. const canExplain = Boolean(onExplain) && Boolean(capabilities?.supportsExplain); @@ -429,65 +431,9 @@ export const QueryEditor = forwardRef( }; } - monacoInstance.editor.defineTheme("db-dark", { - base: "vs-dark", - inherit: true, - rules: [ - { token: "keyword", foreground: "569cd6", fontStyle: "bold" }, - { token: "function", foreground: "dcdcaa" }, - { token: "string", foreground: "ce9178" }, - { token: "number", foreground: "b5cea8" }, - { token: "comment", foreground: "6a9955" }, - { token: "operator", foreground: "d4d4d4" }, - { token: "identifier", foreground: "9cdcfe" }, - ], - colors: { - "editor.background": "#050505", - "editor.foreground": "#d4d4d4", - "editorCursor.foreground": "#569cd6", - "editor.lineHighlightBackground": "#111111", - "editorLineNumber.foreground": "#333333", - "editorLineNumber.activeForeground": "#666666", - "editor.selectionBackground": "#264f78", - "editor.inactiveSelectionBackground": "#3a3d41", - "editorIndentGuide.background": "#1a1a1a", - "editorIndentGuide.activeBackground": "#333333", - }, - }); - - /* - * Monaco paints its own canvas and knows nothing about the CSS token layer, - * so the editor is the one surface that needs the palette written twice. - * Same syntax hues either side — they are chosen for contrast against the - * CODE, not against the chrome — with only the ground and the guides moved. - * `editor.background` mirrors `--studio-canvas` in both themes so the pane - * sits flush with the shell it lives in. - */ - monacoInstance.editor.defineTheme("db-light", { - base: "vs", - inherit: true, - rules: [ - { token: "keyword", foreground: "0000ff", fontStyle: "bold" }, - { token: "function", foreground: "795e26" }, - { token: "string", foreground: "a31515" }, - { token: "number", foreground: "098658" }, - { token: "comment", foreground: "008000" }, - { token: "operator", foreground: "3f3f46" }, - { token: "identifier", foreground: "001080" }, - ], - colors: { - "editor.background": "#f4f4f5", - "editor.foreground": "#27272a", - "editorCursor.foreground": "#0000ff", - "editor.lineHighlightBackground": "#e4e4e7", - "editorLineNumber.foreground": "#a1a1aa", - "editorLineNumber.activeForeground": "#52525b", - "editor.selectionBackground": "#add6ff", - "editor.inactiveSelectionBackground": "#e5ebf1", - "editorIndentGuide.background": "#e4e4e7", - "editorIndentGuide.activeBackground": "#a1a1aa", - }, - }); + // Both themes come from one owner so this mount and the read-only source viewer + // paint identically (#789). + defineStudioThemes(monacoInstance); }; // SQL completion provider diff --git a/src/lib/editor/monaco-theme.ts b/src/lib/editor/monaco-theme.ts new file mode 100644 index 00000000..7cd97776 --- /dev/null +++ b/src/lib/editor/monaco-theme.ts @@ -0,0 +1,91 @@ +import type * as Monaco from "monaco-editor"; + +/** + * The two editor themes, with ONE definition each, shared by every Monaco mount in the app. + * + * They used to be defined inside `QueryEditor`'s `beforeMount`, which is per-mount state: + * `beforeMount` runs for the mount that declares it and for no other, so a second mount that + * does not run that exact callback gets Monaco's stock `vs`/`vs-dark` and sits visibly beside + * a query editor it does not match. The read-only object source viewer (#789) is that second + * mount, and Phase 3's diff preview would be a third. Both import this module and hand it + * their own `monaco` instance. + * + * `editor.defineTheme` registers on the Monaco INSTANCE, not on the mount, and monaco-editor + * 0.56.0 documents it as "Define a new theme or update an existing theme" + * (`monaco-editor/esm/vs/editor/editor.api.d.ts:1124`), so calling this from every mount's + * `beforeMount` rewrites the same two entries with the same payload rather than accumulating + * per-mount state. + */ + +/** Theme id applied whenever the effective app theme is anything but light. */ +export const STUDIO_THEME_DARK = "db-dark"; + +/** Theme id applied when the effective app theme is light. */ +export const STUDIO_THEME_LIGHT = "db-light"; + +/** + * Registers `db-dark` and `db-light` on the Monaco instance handed in. Call it from a mount's + * `beforeMount`, which is the last point before Monaco paints and the first at which an + * instance exists. + */ +export function defineStudioThemes(monacoInstance: typeof Monaco): void { + monacoInstance.editor.defineTheme(STUDIO_THEME_DARK, { + base: "vs-dark", + inherit: true, + rules: [ + { token: "keyword", foreground: "569cd6", fontStyle: "bold" }, + { token: "function", foreground: "dcdcaa" }, + { token: "string", foreground: "ce9178" }, + { token: "number", foreground: "b5cea8" }, + { token: "comment", foreground: "6a9955" }, + { token: "operator", foreground: "d4d4d4" }, + { token: "identifier", foreground: "9cdcfe" }, + ], + colors: { + "editor.background": "#050505", + "editor.foreground": "#d4d4d4", + "editorCursor.foreground": "#569cd6", + "editor.lineHighlightBackground": "#111111", + "editorLineNumber.foreground": "#333333", + "editorLineNumber.activeForeground": "#666666", + "editor.selectionBackground": "#264f78", + "editor.inactiveSelectionBackground": "#3a3d41", + "editorIndentGuide.background": "#1a1a1a", + "editorIndentGuide.activeBackground": "#333333", + }, + }); + + /* + * Monaco paints its own canvas and knows nothing about the CSS token layer, + * so the editor is the one surface that needs the palette written twice. + * Same syntax hues either side — they are chosen for contrast against the + * CODE, not against the chrome — with only the ground and the guides moved. + * `editor.background` mirrors `--studio-canvas` in both themes so the pane + * sits flush with the shell it lives in. + */ + monacoInstance.editor.defineTheme(STUDIO_THEME_LIGHT, { + base: "vs", + inherit: true, + rules: [ + { token: "keyword", foreground: "0000ff", fontStyle: "bold" }, + { token: "function", foreground: "795e26" }, + { token: "string", foreground: "a31515" }, + { token: "number", foreground: "098658" }, + { token: "comment", foreground: "008000" }, + { token: "operator", foreground: "3f3f46" }, + { token: "identifier", foreground: "001080" }, + ], + colors: { + "editor.background": "#f4f4f5", + "editor.foreground": "#27272a", + "editorCursor.foreground": "#0000ff", + "editor.lineHighlightBackground": "#e4e4e7", + "editorLineNumber.foreground": "#a1a1aa", + "editorLineNumber.activeForeground": "#52525b", + "editor.selectionBackground": "#add6ff", + "editor.inactiveSelectionBackground": "#e5ebf1", + "editorIndentGuide.background": "#e4e4e7", + "editorIndentGuide.activeBackground": "#a1a1aa", + }, + }); +} diff --git a/tests/unit/editor/monaco-theme.test.ts b/tests/unit/editor/monaco-theme.test.ts new file mode 100644 index 00000000..c86eba29 --- /dev/null +++ b/tests/unit/editor/monaco-theme.test.ts @@ -0,0 +1,137 @@ +/** + * The two studio editor themes have exactly one definition, and these tests are what + * says so. Before #789 they were defined inside `QueryEditor`'s `beforeMount`, which is + * per-mount state: a second Monaco mount that does not run that callback paints with + * Monaco's stock theme and visibly does not match the query editor beside it. + * + * The full payloads are pinned rather than sampled. A hoist that changes one hex digit + * is a visible regression, and nothing else in the repository would catch it: Monaco + * accepts any colour map, TypeScript accepts any string, and no snapshot covers a + * canvas Monaco paints itself. + */ +import { describe, expect, mock, test } from "bun:test"; +import { defineStudioThemes, STUDIO_THEME_DARK, STUDIO_THEME_LIGHT } from "@/lib/editor/monaco-theme"; + +type DefinedTheme = [string, { base: string; inherit: boolean; rules: unknown[]; colors: Record }]; + +/** A Monaco stand-in that records every `editor.defineTheme` call instead of painting. */ +function createRecordingMonaco() { + const defineTheme = mock((_id: string, _theme: unknown) => {}); + const instance = { editor: { defineTheme } } as never; + return { + instance, + calls: () => defineTheme.mock.calls as unknown as DefinedTheme[], + }; +} + +/** + * Reads one recorded definition by its id rather than by position, and THROWS BY NAME when + * the recorder holds nothing: a test that destructures `calls[0]` of an empty array reports a + * TypeError about `undefined`, which reads like a broken test rather than an undefined theme. + */ +function definitionOf(calls: DefinedTheme[], id: string): DefinedTheme[1] { + if (calls.length === 0) { + throw new Error(`defineStudioThemes defined no theme at all, so "${id}" is missing`); + } + const found = calls.find(([definedId]) => definedId === id); + if (!found) { + throw new Error(`defineStudioThemes never defined "${id}"; it defined ${calls.map(([n]) => n).join(", ")}`); + } + return found[1]; +} + +const DARK_THEME = { + base: "vs-dark", + inherit: true, + rules: [ + { token: "keyword", foreground: "569cd6", fontStyle: "bold" }, + { token: "function", foreground: "dcdcaa" }, + { token: "string", foreground: "ce9178" }, + { token: "number", foreground: "b5cea8" }, + { token: "comment", foreground: "6a9955" }, + { token: "operator", foreground: "d4d4d4" }, + { token: "identifier", foreground: "9cdcfe" }, + ], + colors: { + "editor.background": "#050505", + "editor.foreground": "#d4d4d4", + "editorCursor.foreground": "#569cd6", + "editor.lineHighlightBackground": "#111111", + "editorLineNumber.foreground": "#333333", + "editorLineNumber.activeForeground": "#666666", + "editor.selectionBackground": "#264f78", + "editor.inactiveSelectionBackground": "#3a3d41", + "editorIndentGuide.background": "#1a1a1a", + "editorIndentGuide.activeBackground": "#333333", + }, +}; + +const LIGHT_THEME = { + base: "vs", + inherit: true, + rules: [ + { token: "keyword", foreground: "0000ff", fontStyle: "bold" }, + { token: "function", foreground: "795e26" }, + { token: "string", foreground: "a31515" }, + { token: "number", foreground: "098658" }, + { token: "comment", foreground: "008000" }, + { token: "operator", foreground: "3f3f46" }, + { token: "identifier", foreground: "001080" }, + ], + colors: { + "editor.background": "#f4f4f5", + "editor.foreground": "#27272a", + "editorCursor.foreground": "#0000ff", + "editor.lineHighlightBackground": "#e4e4e7", + "editorLineNumber.foreground": "#a1a1aa", + "editorLineNumber.activeForeground": "#52525b", + "editor.selectionBackground": "#add6ff", + "editor.inactiveSelectionBackground": "#e5ebf1", + "editorIndentGuide.background": "#e4e4e7", + "editorIndentGuide.activeBackground": "#a1a1aa", + }, +}; + +describe("the studio theme ids", () => { + test("are the ids the query editor already shipped, so a hoist does not rename a theme", () => { + expect(STUDIO_THEME_DARK).toBe("db-dark"); + expect(STUDIO_THEME_LIGHT).toBe("db-light"); + }); +}); + +describe("defineStudioThemes", () => { + test("defines both themes on the instance it is given, dark first, and defines nothing else", () => { + const monaco = createRecordingMonaco(); + + defineStudioThemes(monaco.instance); + + expect(monaco.calls().map(([id]) => id)).toEqual([STUDIO_THEME_DARK, STUDIO_THEME_LIGHT]); + }); + + test("defines the dark theme with the payload the query editor shipped, byte for byte", () => { + const monaco = createRecordingMonaco(); + + defineStudioThemes(monaco.instance); + + expect(definitionOf(monaco.calls(), STUDIO_THEME_DARK)).toEqual(DARK_THEME); + }); + + test("defines the light theme with the payload the query editor shipped, byte for byte", () => { + const monaco = createRecordingMonaco(); + + defineStudioThemes(monaco.instance); + + expect(definitionOf(monaco.calls(), STUDIO_THEME_LIGHT)).toEqual(LIGHT_THEME); + }); + + test("gives a second mount the same two definitions, which is the whole reason it exists", () => { + const queryEditorMount = createRecordingMonaco(); + const sourceViewerMount = createRecordingMonaco(); + + defineStudioThemes(queryEditorMount.instance); + defineStudioThemes(sourceViewerMount.instance); + + expect(sourceViewerMount.calls().length).toBe(2); + expect(sourceViewerMount.calls()).toEqual(queryEditorMount.calls()); + }); +}); From 7a2a83d7a1fe2d03a9b1f07d1f8c5770ba670399 Mon Sep 17 00:00:00 2001 From: cevheri Date: Sat, 12 Sep 2026 21:56:14 +0300 Subject: [PATCH 06/94] test(objects): refuse a source-bearing kind expected at zero without a reason (#789) A source-bearing kind the expectation OMITS was already refused by name, but a kind it NAMES AT ZERO was exercised by nothing and passed in silence. Oracle declares nine source-bearing kinds, so an expectation naming eight truthfully and the ninth at zero certified that ninth unread. Requiring a non-zero everywhere would refuse correct fixtures: Cassandra ships materialized views and user-defined functions disabled, and a Trino materialized view needs a connector the compose cluster may not get. So the bar is the reason, in the grammar KindCount already uses for an absence: a zero that says which absence it is, in the engine's or the fixture's own words. assertObjectSurface now throws by name for a source-bearing kind counted at zero with no emptyKinds entry, for a blank or whitespace one, and for a reason naming a kind that is not a source-bearing kind at zero, so a sentence cannot outlive the absence it describes. Four mutations run and four killed. --- tests/helpers/object-surface-conformance.ts | 86 +++++++++++++++++-- .../db/object-surface-conformance.test.ts | 70 +++++++++++++++ 2 files changed, 149 insertions(+), 7 deletions(-) diff --git a/tests/helpers/object-surface-conformance.ts b/tests/helpers/object-surface-conformance.ts index 6bf8057d..a46b021f 100644 --- a/tests/helpers/object-surface-conformance.ts +++ b/tests/helpers/object-surface-conformance.ts @@ -52,11 +52,13 @@ * which is sixteen of the seventeen while the bulk read lands one family at a time. What it * asserts, and why each part of it is not vacuous, is in `assertBulkColumnRead()` below. * - * One further check guards the caller rather than the provider: an expectation naming a + * Two further checks guard the caller rather than the provider. An expectation naming a * kind countObjects never answered for is reported by name. That is a caller-side * mistake, a kind id written into the expectation that this engine never declares, and * without the explicit throw it surfaces as `Cannot use 'in' operator ... in undefined`, - * which names neither the kind nor the expectation. + * which names neither the kind nor the expectation. And a source-bearing kind the + * expectation counts at ZERO must carry a reason in `emptyKinds`, because a zero is the one + * count this contract reads nothing for: see that field's docblock. */ import { expect } from "bun:test"; import { QueryError } from "@/lib/db/errors"; @@ -108,6 +110,39 @@ export interface ObjectSurfaceExpectation { * source-bearing kind, and the helper throws by name when it is missing. */ readonly absentSource?: { readonly path: readonly string[]; readonly kind: string }; + /** + * WHY a source-bearing kind is expected at ZERO, one sentence per kind id. + * + * Required for every source-bearing kind this expectation counts at 0, and the helper + * throws by name without it. A zero is the one count that READS NOTHING: the source loop + * walks the kinds counted above zero, so a kind at zero has its `readObjectSource` path + * driven by no object at all while every assertion around it stays green. Naming a kind at + * zero and naming it truthfully are two different acts, and this field is the second one. + * An expectation that names eight of an engine's nine source-bearing kinds truthfully and + * the ninth at zero certifies that ninth unread. + * + * A non-zero is deliberately NOT demanded instead, because legitimate zeros exist: + * Cassandra ships materialized views and user-defined functions disabled by default, and a + * Trino materialized view needs a connector the compose cluster may not get. So the bar is + * the repository's own grammar for an absence, the one `KindCount` already uses in its + * `{ unavailable }` arm: an absence that says WHICH absence it is, in the engine's or the + * fixture's own words. + * + * Write the FACT, and write it for someone reading a red build who has never seen this + * engine. "the Cassandra image ships with materialized views disabled" is a fact. + * "not applicable", "none" and "TODO" are not, and a blank one is refused outright. + * + * Say which of the two absences it is, because they are different facts and only the + * sentence can tell them apart: the FIXTURE holds none of this kind today, which whoever + * owns the fixture can close by adding one, or this DEPLOYMENT cannot hold one at all, + * which nobody can close here. A fixture shortfall is debt and belongs in the backlog as + * well as here; a deployment that cannot hold one is the end of the matter. One field + * carries both honestly only if you write which one you mean. + * + * A reason for a kind that is NOT a source-bearing kind counted at zero is refused too, so + * a sentence cannot outlive the absence it was written about. + */ + readonly emptyKinds?: Readonly>; } function startsWith(path: readonly string[], prefix: readonly string[]): boolean { @@ -417,6 +452,9 @@ async function assertBulkColumnRead( * - a source-bearing kind the expectation never NAMES is refused by name, one notch narrower * than "named none". Oracle declares nine of them and an expectation naming one would * silence a "none" guard while eight kinds went unread; + * - a source-bearing kind the expectation counts at ZERO is exercised by nothing at all, so + * it must carry a reason in `emptyKinds` saying which absence it is, which is the closest + * a helper can come to reading a kind no object exists for; * - a document of nothing but refusals answers no readable part, so the bound probe below * would never run and every bound assertion would be vacuous; * - a definition under two characters cannot be bounded distinguishably, which is the same @@ -456,12 +494,13 @@ async function assertSourceSurface( // // WHAT THIS LEAVES OPEN, stated so no provider task has to discover it: a source-bearing // kind named at ZERO has its `readObjectSource` path driven by NOTHING here, because there - // is no object of it to read. Only the OMITTED case is refused. On an engine declaring many - // source-bearing kinds, a fixture holding none of one of them therefore drops that kind's - // read out of the contract entirely while every assertion stays green. The remedy is the + // is no object of it to read, and no assertion below can close that. The remedy is the // FIXTURE and not this helper: build one that holds an object of every source-bearing kind - // the engine declares. The `longest === undefined` throw below bounds the damage by - // requiring at least one readable part from at least one kind, and that is all it does. + // the engine declares. What the helper CAN do, and does immediately below, is refuse a zero + // that does not say which absence it is, so a kind dropping out of the contract is a + // sentence a reviewer reads rather than a silence. The `longest === undefined` throw + // further down bounds the damage by requiring at least one readable part from at least one + // kind, and that is all it does. const unexercised = sourceKinds.filter((kind) => !Object.hasOwn(expected.kinds, kind.id)).map((kind) => kind.id); if (unexercised.length > 0) { throw new Error( @@ -470,6 +509,39 @@ async function assertSourceSurface( ); } + // The zero, which is the case the `unexercised` guard above deliberately lets through and + // which nothing else here exercises. A kind counted above zero is read by the loop below; + // a kind the expectation OMITS is refused above; a kind NAMED AT ZERO is read by nothing, + // and until this throw it passed in silence. Requiring a non-zero would refuse correct + // fixtures, so what is required is the reason, in the same grammar `KindCount` uses for an + // absence. What the reason must say is on `emptyKinds` above, and that docblock is the + // instruction every provider task reads. + const reasons = expected.emptyKinds ?? {}; + const zeroed = sourceKinds.filter((kind) => expected.kinds[kind.id] === 0).map((kind) => kind.id); + for (const id of zeroed) { + if (!Object.hasOwn(reasons, id)) { + throw new Error( + `${provider.type} expects zero of the source-bearing kind "${id}", which reads nothing, and emptyKinds ` + + `carries no reason for it; say in emptyKinds["${id}"] whether this fixture holds none of it yet or this ` + + "deployment cannot hold one at all", + ); + } + if (reasons[id].trim() === "") { + throw new Error(`emptyKinds["${id}"] carries no sentence a person can read, which is not a reason`); + } + } + // The other direction, so a sentence cannot outlive the absence it was written about: a + // kind that grew an object, or that never bore source at all, keeps a reason that now + // explains nothing, and a reader trusts it. + for (const id of Object.keys(reasons)) { + if (!zeroed.includes(id)) { + throw new Error( + `emptyKinds names "${id}", which is not a source-bearing kind this expectation counts at zero, so its ` + + "reason describes nothing", + ); + } + } + const wanted = new Set(sourceKinds.filter((kind) => (expected.kinds[kind.id] ?? 0) > 0).map((kind) => kind.id)); const entered = new Set(); let longest: { kind: string; path: readonly string[]; length: number } | undefined; diff --git a/tests/unit/db/object-surface-conformance.test.ts b/tests/unit/db/object-surface-conformance.test.ts index 9a7fd8e8..756a8faf 100644 --- a/tests/unit/db/object-surface-conformance.test.ts +++ b/tests/unit/db/object-surface-conformance.test.ts @@ -982,6 +982,76 @@ describe("assertObjectSurface and the object source read", () => { ); }); + /** + * The zero, which is the one expectation that READS NOTHING (Task 1b, #789). + * + * The four tests below are the whole of it. A source-bearing kind counted above zero is + * read by the loop; a source-bearing kind the expectation OMITS is refused by the test + * above; and between those two sits a kind the expectation NAMES AT ZERO, which is + * exercised by nothing and passed in silence. Oracle declares nine source-bearing kinds, + * so an expectation naming eight truthfully and the ninth at zero certified that ninth + * unread. Requiring a non-zero instead would be wrong, because Cassandra ships + * materialized views and UDFs disabled and a Trino materialized view needs a connector the + * compose cluster may not get, so what is required is the REASON. + * + * `view` is source-bearing and empty in every double here, so the kind under test is + * genuinely at zero rather than made zero by the expectation alone. + */ + function emptyViewProvider(overrides: Record = {}) { + return sourceProvider({ + getCapabilities: () => ({ + ...capabilities(), + objectKinds: capabilities().objectKinds.map((kind) => + kind.id === "view" ? { ...kind, hasSource: true, sourceLanguage: "sql" } : kind, + ), + }), + countObjects: async () => ({ table: { count: 2 }, view: { count: 0 }, function: { count: 2 } }), + listObjects: async (_c: readonly string[], kind: string) => (kind === "view" ? [] : (listed[kind] ?? [])), + ...overrides, + }); + } + + const emptyViewExpectation = { + ...expectation, + kinds: { table: 2, view: 0, function: 2 }, + sampleObject: { path: ["app", "order_total(integer)"], kind: "function" }, + }; + + test("a source-bearing kind expected at zero passes when the expectation says why", async () => { + await assertObjectSurface(emptyViewProvider() as never, { + ...emptyViewExpectation, + emptyKinds: { view: "the fixture defines no view over the two probe tables yet" }, + }); + }); + + test("a source-bearing kind expected at zero with no reason is refused by name", async () => { + await expect(assertObjectSurface(emptyViewProvider() as never, emptyViewExpectation)).rejects.toThrow( + /expects zero of the source-bearing kind "view", which reads nothing, and emptyKinds carries no reason for it/, + ); + }); + + // Whitespace and not only the empty string, because a reason nobody can read is the same + // absence wearing an answer's clothes, which is the bar `assertSourceDocument` already + // holds an engine's own refusal sentence to. + test("a blank reason for a kind expected at zero is refused", async () => { + await expect( + assertObjectSurface(emptyViewProvider() as never, { ...emptyViewExpectation, emptyKinds: { view: " \n " } }), + ).rejects.toThrow(/emptyKinds\["view"\] carries no sentence a person can read/); + }); + + // The other direction, so a sentence cannot outlive the absence it describes: `function` + // holds two objects and is read by the loop, so a reason for it explains nothing. + test("a reason for a kind that is not a source-bearing kind at zero is refused", async () => { + await expect( + assertObjectSurface(emptyViewProvider() as never, { + ...emptyViewExpectation, + emptyKinds: { view: "the fixture defines no view yet", function: "stale, this kind holds two" }, + }), + ).rejects.toThrow( + /emptyKinds names "function", which is not a source-bearing kind this expectation counts at zero/, + ); + }); + test("an absentSource naming a kind the source loop never read has no control, and is refused", async () => { await expect( assertObjectSurface(sourceProvider() as never, { From f4d03b9a76ffdc474f968e91b70963077bf79b76 Mon Sep 17 00:00:00 2001 From: cevheri Date: Sat, 12 Sep 2026 22:13:11 +0300 Subject: [PATCH 07/94] test(objects): check the half of an empty-kind reason a machine can settle (#789) Fix round 1 on the expectation-reason ruling. The emptyKinds docblock is the instruction sixteen provider tasks read, and both of its motivating examples were false about the containers this repository starts: the compose service re-enables Cassandra's materialized views and user-defined functions, and Trino's zero is `view` rather than `materialized_view`. Replaced with the zeros actually committed here, with Cassandra kept as the worked example of writing an engine default where a fixture fact belongs. The reason itself is now checked where it is decidable. A zeroed source-bearing kind is listed, and a sentence claiming an absence the fixture does not have is refused by name with the listing beside it. The four verdict strings the docblock called not-reasons are refused rather than only described. The stale-reason loop runs before the early return for a provider bearing no source, so a sentence cannot survive a dropped hasSource declaration. And `zeroed` is the exact complement of `wanted`, so a count that is neither zero nor positive is held to the same reason instead of falling between both guards. A source-bearing kind whose count is the unavailable arm can be neither named nor omitted, which two providers can already produce; the count refusal now says so instead of leaving the cheap wrong exit of dropping the declaration. Five new tests, nine mutations run and nine killed. --- tests/helpers/object-surface-conformance.ts | 150 ++++++++++++++---- .../db/object-surface-conformance.test.ts | 94 ++++++++++- 2 files changed, 209 insertions(+), 35 deletions(-) diff --git a/tests/helpers/object-surface-conformance.ts b/tests/helpers/object-surface-conformance.ts index a46b021f..7072de4b 100644 --- a/tests/helpers/object-surface-conformance.ts +++ b/tests/helpers/object-surface-conformance.ts @@ -113,24 +113,37 @@ export interface ObjectSurfaceExpectation { /** * WHY a source-bearing kind is expected at ZERO, one sentence per kind id. * - * Required for every source-bearing kind this expectation counts at 0, and the helper - * throws by name without it. A zero is the one count that READS NOTHING: the source loop + * Required for every source-bearing kind this expectation counts at 0, or at anything else + * that is not positive, and the helper throws by name without it. A zero is the one count that READS NOTHING: the source loop * walks the kinds counted above zero, so a kind at zero has its `readObjectSource` path * driven by no object at all while every assertion around it stays green. Naming a kind at * zero and naming it truthfully are two different acts, and this field is the second one. * An expectation that names eight of an engine's nine source-bearing kinds truthfully and * the ninth at zero certifies that ninth unread. * - * A non-zero is deliberately NOT demanded instead, because legitimate zeros exist: - * Cassandra ships materialized views and user-defined functions disabled by default, and a - * Trino materialized view needs a connector the compose cluster may not get. So the bar is - * the repository's own grammar for an absence, the one `KindCount` already uses in its + * A non-zero is deliberately NOT demanded instead, because legitimate zeros exist and this + * repository already commits three: Trino counts `view` at 0 beside a materialized view at + * 1 (`tests/integration/db/trino-provider.test.ts`), and Druid counts `lookup` and + * `system_table` at 0 (`tests/integration/db/druid-provider.test.ts`). So the bar is the + * repository's own grammar for an absence, the one `KindCount` already uses in its * `{ unavailable }` arm: an absence that says WHICH absence it is, in the engine's or the * fixture's own words. * - * Write the FACT, and write it for someone reading a red build who has never seen this - * engine. "the Cassandra image ships with materialized views disabled" is a fact. - * "not applicable", "none" and "TODO" are not, and a blank one is refused outright. + * Write the FACT, for someone reading a red build who has never seen this engine, and write + * it about THE CONTAINER THIS REPOSITORY STARTS rather than about the engine's defaults, + * because the two differ and the compose service is what decides. Cassandra is the worked + * example of getting that wrong: the 5.0 image does ship `materialized_views_enabled` and + * `user_defined_functions_enabled` disabled, so "the Cassandra image ships materialized + * views disabled" reads like a fact and is FALSE here, because the compose service rewrites + * both into `cassandra.yaml` precisely so the fixture measures an engine and not a + * configuration, and the committed expectation counts `materialized_view` at 1 and + * `function` at 4 (`tests/integration/db/cassandra-provider.test.ts`, + * `docs/providers/cassandra.md`). Check the fixture, then write the sentence. + * + * "not applicable", "n/a", "none" and "TODO" are verdicts rather than facts, and all four + * are refused by name, as is a blank one. The helper also asks `listObjects` for the kind, + * so a reason over a kind this fixture demonstrably holds is refused rather than reviewed: + * the half of the sentence that is machine-decidable is decided. * * Say which of the two absences it is, because they are different facts and only the * sentence can tell them apart: the FIXTURE holds none of this kind today, which whoever @@ -187,7 +200,21 @@ export async function assertObjectSurface( for (const [id, want] of Object.entries(expected.kinds)) { if (!(id in counts)) throw new Error(`countObjects returned nothing for expected kind "${id}"`); const got = counts[id]; - if (isCountUnavailable(got)) throw new Error(`kind "${id}" was unavailable: ${got.unavailable}`); + if (isCountUnavailable(got)) { + // A kind that declares `hasSource` and answers the unavailable arm STRUCTURALLY has no + // expectation a task can write: naming it throws here, and omitting it throws as + // unexercised in the source check below. Two providers answer it that way already, so + // the diagnostic names the dead end rather than leaving a wave-4 task to conclude that + // dropping the declaration is the repair. The grammar for it is an open ruling (#789). + const cannotBeWritten = findKind(capabilities, id)?.hasSource === true; + throw new Error( + `kind "${id}" was unavailable: ${got.unavailable}` + + (cannotBeWritten + ? `; and "${id}" declares hasSource, which this expectation shape cannot express, because naming the ` + + "kind throws here and omitting it is refused as unexercised (#789)" + : ""), + ); + } expect(got.count).toBe(want); } @@ -253,7 +280,7 @@ export async function assertObjectSurface( expect(detail.path).toEqual([...sample.path]); await assertBulkColumnRead(provider, container, listings); - await assertSourceSurface(provider, expected, listings); + await assertSourceSurface(provider, expected, container, listings); } /** @@ -433,6 +460,38 @@ async function assertBulkColumnRead( } } +/** + * The strings a reason may NOT be, refused by name rather than only described. + * + * A docblock that enumerates forbidden words while the code accepts them is the weaker half + * of a rule: a task under gate pressure writes `none`, clears every check, and the guard that + * exists to turn a silence into a sentence has produced a different silence. Trimmed and + * case-folded before the lookup, because ` None ` is the same non-answer. + * + * It is a NAMED list and not a test of meaning: a verdict spelled another way still passes, + * and review is what catches that. What this closes is the four spellings a hurried author + * actually reaches for. + */ +const NOT_A_REASON: ReadonlySet = new Set(["not applicable", "n/a", "none", "todo"]); + +/** + * A reason may not outlive the absence it was written about. + * + * Runs in BOTH arms of the source check, including the one that returns early for a provider + * implementing no source read at all, because a reason left behind when a `hasSource` + * declaration is dropped is exactly the sentence a reader would still trust. + */ +function assertNoStaleReason(reasons: Readonly>, zeroed: readonly string[]): void { + for (const id of Object.keys(reasons)) { + if (!zeroed.includes(id)) { + throw new Error( + `emptyKinds names "${id}", which is not a source-bearing kind this expectation counts at zero, so its ` + + "reason describes nothing", + ); + } + } +} + /** * The optional sixth method, checked against the provider's OWN listing (#789 Phase 2). * @@ -452,9 +511,13 @@ async function assertBulkColumnRead( * - a source-bearing kind the expectation never NAMES is refused by name, one notch narrower * than "named none". Oracle declares nine of them and an expectation naming one would * silence a "none" guard while eight kinds went unread; - * - a source-bearing kind the expectation counts at ZERO is exercised by nothing at all, so - * it must carry a reason in `emptyKinds` saying which absence it is, which is the closest - * a helper can come to reading a kind no object exists for; + * - a source-bearing kind the expectation counts at ZERO, or at anything else that is not + * positive, is exercised by nothing at all, so it must carry a reason in `emptyKinds` + * saying which absence it is, which is the closest a helper can come to reading a kind no + * object exists for. The one half of that sentence a machine can decide IS decided: + * `listObjects` is asked, and a reason over a kind the fixture actually holds is refused; + * - a reason is refused in the other direction too, and BEFORE the early return for a + * provider bearing no source, so a sentence cannot outlive the absence it describes; * - a document of nothing but refusals answers no readable part, so the bound probe below * would never run and every bound assertion would be vacuous; * - a definition under two characters cannot be bounded distinguishably, which is the same @@ -463,6 +526,7 @@ async function assertBulkColumnRead( async function assertSourceSurface( provider: DatabaseProvider, expected: ObjectSurfaceExpectation, + container: readonly string[], listings: ReadonlyMap, ): Promise { const capabilities = provider.getCapabilities(); @@ -477,7 +541,15 @@ async function assertSourceSurface( `${typeof read === "function" ? "implements readObjectSource" : "does not implement readObjectSource"}`, ); } - if (read === undefined) return; + // BEFORE the early return, which is where this guard was wrong: a provider bearing no + // source at all left every reason unread, so a task that dropped a `hasSource` declaration + // and kept the sentence was not refused. `sourceKinds` is empty here by the pairing above, + // so every reason is stale by construction. + const reasons = expected.emptyKinds ?? {}; + if (read === undefined) { + assertNoStaleReason(reasons, []); + return; + } const absent = expected.absentSource; if (absent === undefined) { @@ -498,7 +570,10 @@ async function assertSourceSurface( // FIXTURE and not this helper: build one that holds an object of every source-bearing kind // the engine declares. What the helper CAN do, and does immediately below, is refuse a zero // that does not say which absence it is, so a kind dropping out of the contract is a - // sentence a reviewer reads rather than a silence. The `longest === undefined` throw + // sentence a reviewer reads rather than a silence, and ask `listObjects` whether that + // sentence is true, which is the one half of it a machine can settle. The half it cannot is + // a fixture author's honest sentence against a convenient one about a kind the fixture + // really is empty of, and that stays a REVIEW obligation. The `longest === undefined` throw // further down bounds the damage by requiring at least one readable part from at least one // kind, and that is all it does. const unexercised = sourceKinds.filter((kind) => !Object.hasOwn(expected.kinds, kind.id)).map((kind) => kind.id); @@ -516,31 +591,46 @@ async function assertSourceSurface( // fixtures, so what is required is the reason, in the same grammar `KindCount` uses for an // absence. What the reason must say is on `emptyKinds` above, and that docblock is the // instruction every provider task reads. - const reasons = expected.emptyKinds ?? {}; - const zeroed = sourceKinds.filter((kind) => expected.kinds[kind.id] === 0).map((kind) => kind.id); + // + // NOT `=== 0`: the predicate is the exact complement of `wanted` below, because a count + // that is neither zero nor positive is read by nothing either and a `=== 0` test let it + // through both guards unexplained. `unexercised` above has already refused every + // source-bearing kind the expectation does not name, so every id reaching here is named. + const zeroed = sourceKinds.filter((kind) => !(expected.kinds[kind.id] > 0)).map((kind) => kind.id); for (const id of zeroed) { if (!Object.hasOwn(reasons, id)) { throw new Error( - `${provider.type} expects zero of the source-bearing kind "${id}", which reads nothing, and emptyKinds ` + - `carries no reason for it; say in emptyKinds["${id}"] whether this fixture holds none of it yet or this ` + - "deployment cannot hold one at all", + `${provider.type} counts the source-bearing kind "${id}" at ${expected.kinds[id]}, which reads nothing, ` + + `and emptyKinds carries no reason for it; say in emptyKinds["${id}"] whether this fixture holds none of ` + + "it yet or this deployment cannot hold one at all", ); } - if (reasons[id].trim() === "") { + const reason = reasons[id].trim(); + if (reason === "") { throw new Error(`emptyKinds["${id}"] carries no sentence a person can read, which is not a reason`); } - } - // The other direction, so a sentence cannot outlive the absence it was written about: a - // kind that grew an object, or that never bore source at all, keeps a reason that now - // explains nothing, and a reader trusts it. - for (const id of Object.keys(reasons)) { - if (!zeroed.includes(id)) { + if (NOT_A_REASON.has(reason.toLowerCase())) { throw new Error( - `emptyKinds names "${id}", which is not a source-bearing kind this expectation counts at zero, so its ` + - "reason describes nothing", + `emptyKinds["${id}"] is the verdict "${reason}", which states no fact; write what is absent and why, in ` + + "the fixture's or the engine's own words", + ); + } + // The half of the sentence's truthfulness a machine CAN decide. The listing loop skips a + // kind counted at zero, so nothing else here ever asks the provider whether the fixture + // really holds none of it, and a count and a listing that disagree for one kind (which + // this helper tolerates in MAGNITUDE on purpose, two reads at two instants) would let a + // written claim of absence stand over an object the fixture demonstrably holds. Zero + // against non-empty is not a magnitude. + const listed = await provider.listObjects!(container, id); + if (listed.length > 0) { + throw new Error( + `emptyKinds["${id}"] explains an absence (${JSON.stringify(reason)}) while listObjects("${id}") returned ` + + `${listed.length} object(s), the first at ${JSON.stringify(listed[0].path)}, so the fixture holds the ` + + "kind the reason says it does not", ); } } + assertNoStaleReason(reasons, zeroed); const wanted = new Set(sourceKinds.filter((kind) => (expected.kinds[kind.id] ?? 0) > 0).map((kind) => kind.id)); const entered = new Set(); diff --git a/tests/unit/db/object-surface-conformance.test.ts b/tests/unit/db/object-surface-conformance.test.ts index 756a8faf..ec2f6ce4 100644 --- a/tests/unit/db/object-surface-conformance.test.ts +++ b/tests/unit/db/object-surface-conformance.test.ts @@ -985,14 +985,16 @@ describe("assertObjectSurface and the object source read", () => { /** * The zero, which is the one expectation that READS NOTHING (Task 1b, #789). * - * The four tests below are the whole of it. A source-bearing kind counted above zero is + * The eight tests below are the whole of it. A source-bearing kind counted above zero is * read by the loop; a source-bearing kind the expectation OMITS is refused by the test * above; and between those two sits a kind the expectation NAMES AT ZERO, which is * exercised by nothing and passed in silence. Oracle declares nine source-bearing kinds, * so an expectation naming eight truthfully and the ninth at zero certified that ninth - * unread. Requiring a non-zero instead would be wrong, because Cassandra ships - * materialized views and UDFs disabled and a Trino materialized view needs a connector the - * compose cluster may not get, so what is required is the REASON. + * unread. Requiring a non-zero instead would be wrong, because legitimate zeros are already + * committed here: Trino counts `view` at 0 and Druid counts `lookup` and `system_table` at + * 0. So what is required is the REASON, and the reason is held to four bars: it exists, it + * is not blank, it is not one of four named verdicts, and `listObjects` agrees the fixture + * really holds none of the kind. * * `view` is source-bearing and empty in every double here, so the kind under test is * genuinely at zero rather than made zero by the expectation alone. @@ -1026,7 +1028,7 @@ describe("assertObjectSurface and the object source read", () => { test("a source-bearing kind expected at zero with no reason is refused by name", async () => { await expect(assertObjectSurface(emptyViewProvider() as never, emptyViewExpectation)).rejects.toThrow( - /expects zero of the source-bearing kind "view", which reads nothing, and emptyKinds carries no reason for it/, + /counts the source-bearing kind "view" at 0, which reads nothing, and emptyKinds carries no reason for it/, ); }); @@ -1052,6 +1054,88 @@ describe("assertObjectSurface and the object source read", () => { ); }); + // Fix round 1, finding 3: the one half of the reason's truthfulness a machine CAN decide. + // The listing loop skips a kind counted at zero, so a count and a listing that disagree + // for that kind let a sentence saying the fixture holds none of it stand beside a listing + // that holds one. The helper tolerates count/listing disagreement in magnitude on purpose + // (two reads at two instants), but zero against non-empty is not a magnitude. + test("a reason for a kind listObjects actually returns objects for is refused", async () => { + await expect( + assertObjectSurface( + emptyViewProvider({ + listObjects: async (_c: readonly string[], kind: string) => + kind === "view" ? [{ path: ["app", "order_summary"], name: "order_summary", kind: "view" }] : listed[kind], + }) as never, + { ...emptyViewExpectation, emptyKinds: { view: "the fixture defines no view yet" } }, + ), + ).rejects.toThrow(/while listObjects\("view"\) returned 1 object\(s\), the first at \["app","order_summary"\]/); + }); + + // Fix round 1, finding 5: the docblock named three strings as not-reasons and the helper + // refused none of them, so a task under gate pressure could write `none` and turn a + // silence into a different silence. Trimmed and case-folded, because ` None ` is the + // same non-answer. + test("a verdict rather than a fact is refused for a kind expected at zero", async () => { + for (const verdict of ["none", " N/A ", "Not Applicable", "TODO"]) { + await expect( + assertObjectSurface(emptyViewProvider() as never, { ...emptyViewExpectation, emptyKinds: { view: verdict } }), + ).rejects.toThrow(/states no fact; write what is absent and why/); + } + }); + + // Fix round 1, finding 6: the stale-reason guard sat AFTER the `readObjectSource === undefined` + // return, so a reason on a provider bearing no source at all was ignored rather than refused. + // That is the exact state a provider task reaches by dropping a `hasSource` declaration and + // leaving the sentence behind. + test("a reason on a provider that bears no source at all is refused, not ignored", async () => { + const noSource = sourceProvider({ + getCapabilities: () => capabilities({ hasSource: undefined, sourceLanguage: undefined }), + readObjectSource: undefined, + }); + await expect( + assertObjectSurface(noSource as never, { + ...expectation, + emptyKinds: { view: "a sentence about a kind that bears no source" }, + }), + ).rejects.toThrow(/emptyKinds names "view", which is not a source-bearing kind this expectation counts at zero/); + }); + + // Fix round 1, finding 7: `zeroed` tested `=== 0` while `wanted` tested `> 0`, so a count + // that is neither left a source-bearing kind unread AND unexplained. The two predicates are + // exact complements now, and the message carries the count rather than the word zero. + test("a source-bearing kind counted below zero is held to the same reason", async () => { + // Listing an object for `view` is the reviewer's own probe shape: with an empty listing + // the loop above refuses the negative count first, and it is the pair that reached the + // source half unread. + const negative = emptyViewProvider({ + countObjects: async () => ({ table: { count: 2 }, view: { count: -1 }, function: { count: 2 } }), + listObjects: async (_c: readonly string[], kind: string) => + kind === "view" ? [{ path: ["app", "order_summary"], name: "order_summary", kind: "view" }] : listed[kind], + }); + await expect( + assertObjectSurface(negative as never, { ...emptyViewExpectation, kinds: { table: 2, view: -1, function: 2 } }), + ).rejects.toThrow(/counts the source-bearing kind "view" at -1, which reads nothing/); + }); + + // Fix round 1, finding 8, escalated to the orchestrator in the report and given a + // diagnostic here so a wave-4 task meets a named gap rather than a puzzle. A kind that + // declares `hasSource` and whose count is the `{ unavailable }` arm (cassandra/objects.ts + // and trino/index.ts both answer it structurally) can be neither NAMED, which throws in the + // count loop, nor OMITTED, which throws as unexercised, so the expectation is + // unsatisfiable and the cheap repair under gate pressure is to drop the declaration. + test("a source-bearing kind whose count is unavailable says why the expectation cannot be written", async () => { + const unavailable = sourceProvider({ + countObjects: async () => ({ + table: { count: 2 }, + view: { count: 1 }, + function: { unavailable: 'Cassandra has no statement that lists the kind "function"' }, + }), + }); + await expect(assertObjectSurface(unavailable as never, expectation)).rejects.toThrow( + /declares hasSource, which this expectation shape cannot express/, + ); + }); + test("an absentSource naming a kind the source loop never read has no control, and is refused", async () => { await expect( assertObjectSurface(sourceProvider() as never, { From 7a6dba9352153b6bc594fd945cc3584ac33a5eba Mon Sep 17 00:00:00 2001 From: cevheri Date: Sat, 12 Sep 2026 22:25:55 +0300 Subject: [PATCH 08/94] feat(ui): add the object source caption, the sentence that says what a definition is (#789) The caption is not decoration. Without it a PostgreSQL pg_get_viewdef answer, which is a bare SELECT with no CREATE VIEW in front of it, reads as a complete statement a user could copy and run, and a DuckDB macro body reads as the bytes somebody typed rather than the engine's regeneration from its catalog. A pure exported function rather than JSX, per standing ruling 5b after the happy-dom clamp finding: happy-dom returns zeros for layout, so copy reached through a rendered element can be asserted vacuously. All six compositions are pinned with no DOM at all. Two frozen records rather than one six-cell table, because the two axes are independent and no engine couples them: PostgreSQL produces regenerated in both forms, Couchbase produces partial from a rendered origin. --- .../object-source/source-caption.ts | 42 ++++++++++++++ .../components/object-source-caption.test.ts | 58 +++++++++++++++++++ 2 files changed, 100 insertions(+) create mode 100644 src/components/object-source/source-caption.ts create mode 100644 tests/unit/components/object-source-caption.test.ts diff --git a/src/components/object-source/source-caption.ts b/src/components/object-source/source-caption.ts new file mode 100644 index 00000000..80b748e9 --- /dev/null +++ b/src/components/object-source/source-caption.ts @@ -0,0 +1,42 @@ +import type { ObjectSourceForm, ObjectSourceOrigin } from "@/lib/db/types"; + +/** + * The one sentence that says what a definition on screen IS (#789). + * + * NOT decoration. Two measured readings go wrong without it, and both are silent: a + * PostgreSQL `pg_get_viewdef` answer is a bare SELECT with no `CREATE VIEW` in front of it and + * reads as a complete statement a user could copy and run, and a DuckDB macro body is the + * engine's regeneration from its catalog rather than the bytes anybody typed, which reads as + * the user's own text. The caption is what separates those from an SQL Server module, which is + * genuinely the author's stored bytes run as given. + * + * A PURE exported function, not JSX and not a hook, and that is standing ruling 5b's + * prescription rather than a style choice: happy-dom returns ZEROS for layout and a test that + * reaches this copy through a rendered element asserts less than it appears to. All six + * compositions are pinned in `tests/unit/components/object-source-caption.test.ts` with no DOM + * at all. + * + * TWO INDEPENDENT AXES and therefore two records rather than one six-cell table. `origin` says + * where the bytes came from and `form` says whether they run as given, and no engine couples + * the two: PostgreSQL produces `regenerated` in both forms (a view is `partial`, a function is + * `complete`), and Couchbase produces `partial` from a `rendered` origin. A keyed table would + * be six literals to keep in step for a fact that is three plus two. + */ + +/** Where the bytes came from. One sentence per arm of `ObjectSourceOrigin`, all three used. */ +const ORIGIN_SENTENCE: Readonly> = Object.freeze({ + stored: "Stored by the engine as it was submitted.", + regenerated: "Rebuilt by the engine from its catalog.", + rendered: "A structured definition, rendered here as JSON.", +}); + +/** Whether the bytes run as given. One clause per arm of `ObjectSourceForm`, both used. */ +const FORM_CLAUSE: Readonly> = Object.freeze({ + complete: "Complete as shown.", + partial: "This is the body only, not a complete statement.", +}); + +/** The caption for one part, composed origin first and form second. */ +export function sourceCaption(form: ObjectSourceForm, origin: ObjectSourceOrigin): string { + return `${ORIGIN_SENTENCE[origin]} ${FORM_CLAUSE[form]}`; +} diff --git a/tests/unit/components/object-source-caption.test.ts b/tests/unit/components/object-source-caption.test.ts new file mode 100644 index 00000000..7ec7874f --- /dev/null +++ b/tests/unit/components/object-source-caption.test.ts @@ -0,0 +1,58 @@ +import { describe, expect, test } from "bun:test"; +import { sourceCaption } from "@/components/object-source/source-caption"; + +/** + * The caption is the load-bearing half of the source pane (#789). + * + * It is what stops a `pg_get_viewdef` fragment reading as a complete statement and a DuckDB + * regeneration reading as the user's own text, so all six compositions are pinned here and + * each of the six has at least one producer in the shipped fleet. + * + * A PURE function tested with NO DOM at all, which is standing ruling 5b's prescription after + * the happy-dom clamp finding: happy-dom returns zeros for layout, so a test that reaches the + * copy through a rendered element can assert less than it looks like. + */ +describe("sourceCaption", () => { + test.each([ + ["complete", "stored", "Stored by the engine as it was submitted. Complete as shown."], + ["partial", "stored", "Stored by the engine as it was submitted. This is the body only, not a complete statement."], + ["complete", "regenerated", "Rebuilt by the engine from its catalog. Complete as shown."], + [ + "partial", + "regenerated", + "Rebuilt by the engine from its catalog. This is the body only, not a complete statement.", + ], + ["complete", "rendered", "A structured definition, rendered here as JSON. Complete as shown."], + [ + "partial", + "rendered", + "A structured definition, rendered here as JSON. This is the body only, not a complete statement.", + ], + ] as const)("%s %s", (form, origin, expected) => { + expect(sourceCaption(form, origin)).toBe(expected); + }); + + /** + * The two axes are INDEPENDENT, and this is the assertion that says so rather than trusting + * six literals to imply it. A lookup keyed on the pair would satisfy the six cases above and + * would be a table with six cells to keep in step; the composition is two records of three + * and two entries. Deleting either half of the composition kills every case here, and a + * table would still pass its own six. + */ + test("the origin sentence leads and the form clause follows, for every pair", () => { + const origins = ["stored", "regenerated", "rendered"] as const; + const forms = ["complete", "partial"] as const; + let checked = 0; + for (const origin of origins) { + const complete = sourceCaption("complete", origin); + for (const form of forms) { + const caption = sourceCaption(form, origin); + // Same leading sentence whatever the form: the origin half cannot depend on the form. + expect(caption.startsWith(complete.slice(0, complete.indexOf(". ") + 1))).toBe(true); + checked += 1; + } + } + // Non-vacuity: six pairs were walked, not zero. A loop over an empty list certifies nothing. + expect(checked).toBe(6); + }); +}); From e69978e18103891dd13a8c0b83e26200993eaeb6 Mon Sep 17 00:00:00 2001 From: cevheri Date: Sat, 12 Sep 2026 22:27:51 +0300 Subject: [PATCH 09/94] feat(ui): add the object source read seam and its client shape check (#789) The source read is its OWN seam rather than a fourth member of the tree's ObjectReadRequest. Measured: that union is paired with a ReadSlot whose three kinds each land in a TreeCache map, isRenderableShape dispatches on the slot and not on the route, and the surface that wants a source document is a tab holding no handle on the tree's private source at all. A fourth member would also oblige the embedded adapter's exhaustive switch to carry an arm for a state the design says cannot occur, which is the deleted 501 in a new place under a coverage gate. isSourceDocumentShape is the live home of the invariants TypeScript cannot hold. Four of its rules are about two facts collapsing into one rather than about malformed data: a part carrying both text and unavailable narrows to the refusal and drops the definition in silence, and our own compiler admits that literal because the excess-property check on a union accepts a property declared on any member; a refusal with a blank sentence draws our headline over an empty line; an empty text is not a definition; and a truncation mark with no reason is a warning banner with nothing in it. Duplicate part ids are refused for the switcher's sake, since activePartId addresses a part by id. Nine mutations run and nine killed. --- src/components/object-source/source-reader.ts | 120 ++++++++++++ .../components/object-source-reader.test.ts | 181 ++++++++++++++++++ 2 files changed, 301 insertions(+) create mode 100644 src/components/object-source/source-reader.ts create mode 100644 tests/unit/components/object-source-reader.test.ts diff --git a/src/components/object-source/source-reader.ts b/src/components/object-source/source-reader.ts new file mode 100644 index 00000000..7fe5b10b --- /dev/null +++ b/src/components/object-source/source-reader.ts @@ -0,0 +1,120 @@ +import { appFetch } from "@/lib/config/base-path"; +import { buildConnectionPayload } from "@/hooks/use-connection-payload"; +import type { ObjectSourceDocument, ObjectSourceForm, ObjectSourceOrigin } from "@/lib/db/types"; +import type { DatabaseConnection } from "@/lib/types"; + +/** + * Who answers a source read, and what a renderer is allowed to believe about the answer (#789). + * + * This is its OWN seam and not a fourth member of the tree's `ObjectReadRequest`, which is a + * measured distinction rather than a preference. That type is paired with a `ReadSlot` whose + * three kinds each land in a `TreeCache` map, and `isRenderableShape` dispatches on the slot + * and not on the route; a source document is not an array, it caches nothing, and the surface + * that wants it is a TAB holding no handle on the tree's private source at all. Adding a fourth + * member would also oblige the embedded adapter's exhaustive switch to carry an arm for a state + * the design says cannot occur, which is the deleted 501 in a new place under a coverage gate. + * + * The return is `unknown` on purpose, for both shells rather than for the embedded one alone: a + * route's body and a host callback's return value are both ordinary values this component is + * about to dereference, and only one of them has a type declaration. + */ +export type ObjectSourceReader = ( + connection: DatabaseConnection, + path: readonly string[], + kind: string, +) => Promise; + +/** + * The default source: this application's own route. + * + * `buildConnectionPayload` sends a managed seed by id and anything else in full, which is how + * every other db route is called and the only way a connection the server has never heard of + * can be read at all. + */ +export const httpSourceReader: ObjectSourceReader = async (connection, path, kind) => { + const response = await appFetch("/api/db/objects/source", { + method: "POST", + headers: { "Content-Type": "application/json" }, + body: JSON.stringify({ ...buildConnectionPayload(connection), path, kind }), + }); + // A route that answered with no body at all still answered something worth showing, so the + // status stands in for the sentence rather than the read being reported as a parse error. + const body = (await response.json().catch(() => ({}))) as { error?: string }; + if (!response.ok) { + throw new Error(body.error ?? `The source read failed with HTTP ${response.status}`); + } + return body; +}; + +const FORMS: readonly string[] = ["complete", "partial"] satisfies readonly ObjectSourceForm[]; +const ORIGINS: readonly string[] = ["stored", "regenerated", "rendered"] satisfies readonly ObjectSourceOrigin[]; + +function isRecord(value: unknown): value is Record { + return typeof value === "object" && value !== null && !Array.isArray(value); +} + +/** A string that carries a fact, rather than one that is present and says nothing. */ +function isFilledString(value: unknown): value is string { + return typeof value === "string" && value.trim() !== ""; +} + +/** + * The truncation mark, checked because the banner DEREFERENCES `reason` and prints it. + * + * A mark whose reason is missing would draw an empty warning banner above a text, which is a + * second spelling of the collapse this whole design exists to prevent: an attention state with + * nothing in it reads as decoration. + */ +function isTruncationShape(value: unknown): boolean { + return isRecord(value) && typeof value.limit === "number" && isFilledString(value.reason); +} + +function isPartShape(part: unknown): boolean { + if (!isRecord(part)) return false; + if (!isFilledString(part.id)) return false; + if (!isFilledString(part.label)) return false; + // Both keys at once is the collapse, and it is checked BEFORE either arm is examined, + // because each arm on its own would accept the part. + if (Object.hasOwn(part, "unavailable") && Object.hasOwn(part, "text")) return false; + if (Object.hasOwn(part, "unavailable")) return isFilledString(part.unavailable); + if (!isFilledString(part.text)) return false; + if (!isFilledString(part.language)) return false; + if (!FORMS.includes(part.form as string)) return false; + if (!ORIGINS.includes(part.origin as string)) return false; + if (Object.hasOwn(part, "truncated") && !isTruncationShape(part.truncated)) return false; + return true; +} + +/** + * The client's shape check, and the LIVE home of the invariants the compiler cannot hold. + * + * A predicate rather than a boolean, unlike `isRenderableShape`, so the caller narrows instead + * of casting. Written here because the embedded shell's document comes from a HOST: ordinary + * JavaScript whose declared return type is not a runtime guarantee. It is live for the + * standalone route too, where the body is JSON nobody typed. + * + * Four of these checks are not about malformed data at all, they are about two facts + * collapsing into one: + * - a part carrying BOTH keys narrows to the refusal and drops the text in silence, and + * MEASURED against tsc 6.0.3 our own compiler admits that literal, because TypeScript's + * excess-property check on a union accepts any property declared on any member of it; + * - a refusal with an empty sentence draws our headline over a blank line, which is the + * empty-versus-unreadable collapse this whole design exists to prevent, one level in; + * - an empty text is not a definition, and an editor holding one is the DBeaver shape, + * measured in its source: an unreadable definition in a WRITABLE editor holding one line; + * - a truncation mark with no reason is a warning banner with nothing in it. + * + * Two parts sharing one id is rejected for a different reason, and it is the switcher's: + * `activePartId` addresses a part by id, so two parts under one id make the selection + * unresolvable and a click on the second tab select the first. + */ +export function isSourceDocumentShape(value: unknown): value is ObjectSourceDocument { + if (!isRecord(value)) return false; + if (!Array.isArray(value.path) || !value.path.every((segment) => typeof segment === "string")) return false; + if (typeof value.kind !== "string") return false; + if (!Array.isArray(value.parts) || value.parts.length === 0) return false; + if (!value.parts.every(isPartShape)) return false; + const ids = new Set((value.parts as Record[]).map((part) => part.id as string)); + if (ids.size !== value.parts.length) return false; + return true; +} diff --git a/tests/unit/components/object-source-reader.test.ts b/tests/unit/components/object-source-reader.test.ts new file mode 100644 index 00000000..4a919450 --- /dev/null +++ b/tests/unit/components/object-source-reader.test.ts @@ -0,0 +1,181 @@ +import { afterEach, describe, expect, test } from "bun:test"; +import { httpSourceReader, isSourceDocumentShape } from "@/components/object-source/source-reader"; +import type { DatabaseConnection } from "@/lib/types"; + +/** + * The client's shape check and the standalone reader (#789). + * + * `isSourceDocumentShape` is the LIVE home of the invariants the compiler cannot hold. Three + * of the cases below are not about malformed data at all, they are about two facts collapsing + * into one, and every one of them is reachable from a host: the embedded shell's document + * comes from ordinary JavaScript outside our compiler, where a declared return type is not a + * runtime guarantee. + */ + +const readable = { + id: "definition", + label: "Definition", + text: "SELECT 1", + language: "sql", + form: "complete", + origin: "stored", +}; +const document = { path: ["app", "v"], kind: "view", parts: [readable] }; + +describe("isSourceDocumentShape", () => { + test("accepts a document a provider of ours would build", () => { + expect(isSourceDocumentShape(document)).toBe(true); + }); + + test("accepts a refusal part carrying only the engine's sentence", () => { + expect( + isSourceDocumentShape({ ...document, parts: [{ id: "d", label: "D", unavailable: "It is wrapped." }] }), + ).toBe(true); + }); + + test("accepts a readable part carrying a truncation mark", () => { + expect( + isSourceDocumentShape({ + ...document, + parts: [{ ...readable, truncated: { limit: 10, reason: "bounded at 10 characters" } }], + }), + ).toBe(true); + }); + + test("accepts two parts with distinct ids, which is the Oracle package shape", () => { + expect(isSourceDocumentShape({ ...document, parts: [readable, { ...readable, id: "body" }] })).toBe(true); + }); + + test("rejects a value that is not a record at all", () => { + expect(isSourceDocumentShape(null)).toBe(false); + expect(isSourceDocumentShape([document])).toBe(false); + expect(isSourceDocumentShape("a document")).toBe(false); + }); + + test("rejects a document with no parts, because there would be nothing to draw", () => { + expect(isSourceDocumentShape({ ...document, parts: [] })).toBe(false); + }); + + test("rejects parts that are not an array", () => { + expect(isSourceDocumentShape({ ...document, parts: { 0: readable } })).toBe(false); + }); + + test("rejects a part that is not a record", () => { + expect(isSourceDocumentShape({ ...document, parts: [null] })).toBe(false); + }); + + test("rejects a part carrying BOTH text and unavailable, because the text would be dropped in silence", () => { + // `isSourcePartUnavailable` asks `"unavailable" in part`, so such a part narrows to the + // refusal and the editor never sees the text. A host can build one; our compiler cannot, + // and TypeScript's excess-property check on a union admits the key on either arm. + expect(isSourceDocumentShape({ ...document, parts: [{ ...readable, unavailable: "nope" }] })).toBe(false); + }); + + test("rejects a refusal with an empty sentence, which would draw our headline over a blank line", () => { + expect(isSourceDocumentShape({ ...document, parts: [{ id: "d", label: "D", unavailable: " " }] })).toBe(false); + }); + + test("rejects an empty text, because an empty definition is not a definition", () => { + expect(isSourceDocumentShape({ ...document, parts: [{ ...readable, text: " " }] })).toBe(false); + expect(isSourceDocumentShape({ ...document, parts: [{ ...readable, text: 7 }] })).toBe(false); + }); + + test("rejects a blank id or a blank label, which the switcher could not address or name", () => { + expect(isSourceDocumentShape({ ...document, parts: [{ ...readable, id: " " }] })).toBe(false); + expect(isSourceDocumentShape({ ...document, parts: [{ ...readable, label: "" }] })).toBe(false); + }); + + test("rejects two parts sharing one id, because the switcher could not address either", () => { + expect(isSourceDocumentShape({ ...document, parts: [readable, { ...readable, text: "SELECT 2" }] })).toBe(false); + }); + + test("rejects a blank language, because the editor would be handed nothing to resolve", () => { + expect(isSourceDocumentShape({ ...document, parts: [{ ...readable, language: " " }] })).toBe(false); + }); + + test("rejects a form or an origin outside its union", () => { + expect(isSourceDocumentShape({ ...document, parts: [{ ...readable, form: "whole" }] })).toBe(false); + expect(isSourceDocumentShape({ ...document, parts: [{ ...readable, origin: "typed" }] })).toBe(false); + }); + + test("rejects a truncation mark that is not a limit and a reason", () => { + expect(isSourceDocumentShape({ ...document, parts: [{ ...readable, truncated: { limit: 10 } }] })).toBe(false); + expect( + isSourceDocumentShape({ ...document, parts: [{ ...readable, truncated: { limit: "10", reason: "r" } }] }), + ).toBe(false); + expect(isSourceDocumentShape({ ...document, parts: [{ ...readable, truncated: null }] })).toBe(false); + }); + + test("rejects a path that is not an array of strings", () => { + expect(isSourceDocumentShape({ ...document, path: ["app", null] })).toBe(false); + expect(isSourceDocumentShape({ ...document, path: "app.v" })).toBe(false); + }); + + test("rejects a kind that is not a string", () => { + expect(isSourceDocumentShape({ ...document, kind: 7 })).toBe(false); + }); +}); + +const connection: DatabaseConnection = { + id: "pg-1", + name: "conn", + type: "postgres", + createdAt: new Date("2026-01-01"), +}; + +const realFetch = globalThis.fetch; +afterEach(() => { + globalThis.fetch = realFetch; +}); + +describe("httpSourceReader", () => { + test("posts the connection payload beside the address, and answers the parsed body", async () => { + let seenUrl = ""; + let seenBody: unknown; + globalThis.fetch = (async (url: string, init: RequestInit) => { + seenUrl = String(url); + seenBody = JSON.parse(String(init.body)); + return new Response(JSON.stringify(document), { status: 200 }); + }) as unknown as typeof fetch; + + const answered = await httpSourceReader(connection, ["app", "v"], "view"); + + expect(seenUrl.endsWith("/api/db/objects/source")).toBe(true); + expect(seenBody).toEqual({ + connection: { ...connection, createdAt: connection.createdAt.toISOString() }, + path: ["app", "v"], + kind: "view", + }); + expect(answered).toEqual(document); + }); + + test("sends a managed connection by its seed id and never its credentials", async () => { + let seenBody: Record = {}; + globalThis.fetch = (async (_url: string, init: RequestInit) => { + seenBody = JSON.parse(String(init.body)); + return new Response(JSON.stringify(document), { status: 200 }); + }) as unknown as typeof fetch; + + await httpSourceReader({ ...connection, managed: true, seedId: "demo", password: "hunter2" }, ["app", "v"], "view"); + + expect(seenBody.connectionId).toBe("seed:demo"); + expect(seenBody.connection).toBeUndefined(); + }); + + test("raises the route's own sentence when the read is refused", async () => { + globalThis.fetch = (async () => + new Response(JSON.stringify({ error: "Object app.v was not found." }), { + status: 400, + })) as unknown as typeof fetch; + + await expect(httpSourceReader(connection, ["app", "v"], "view")).rejects.toThrow("Object app.v was not found."); + }); + + test("stands the status in for a sentence when the route answered no body at all", async () => { + globalThis.fetch = (async () => new Response("502", { status: 502 })) as unknown as typeof fetch; + + await expect(httpSourceReader(connection, ["app", "v"], "view")).rejects.toThrow( + "The source read failed with HTTP 502", + ); + }); +}); From d5d9fb9706db61666251b30c4aed026c7b1d1c86 Mon Sep 17 00:00:00 2001 From: cevheri Date: Sat, 12 Sep 2026 22:28:55 +0300 Subject: [PATCH 10/94] feat(api): add the object source route (#789) POST /api/db/objects/source is the seventh route under that prefix, built on handleObjectRequest so it inherits auth-before-parse, rate limiting, connection resolution and error mapping with no new line of any of them. Two exported helpers carry the parts that are its own. requireSourceReader is one branch with two conjuncts: a kind that declares no source, and a provider that declares one and implements no method, which readObjectSource being optional makes representable. Both are reachable and both are pinned by the refusal SENTENCE rather than the status, because a provider raising its own error also maps to 400. It is a helper rather than an inline throw because ObjectRouteError is module-private, and keeping it private is what keeps the status vocabulary in one file. boundSourceDocument applies the character bound to the ANSWER instead of trusting the number the route passed the provider, on the inventory route's precedent: sixteen providers plus any host implementing the embedded seam sit behind it, and the route serialises the whole document in one response. A provider that bounded correctly is returned unchanged; one that over-answered is sliced and its own sentence is kept and joined, because a second bound is a second fact. parts.length is bounded too, since the tuple type has no upper bound. The rate-limit query-bucket census is re-measured rather than incremented: it claimed twenty routes and named four schema routes that no longer exist, while never mentioning the object routes that had joined. Twenty-three handlers today, sixteen direct call sites and seven through object-route.ts. --- src/app/api/db/objects/source/route.ts | 44 +++ src/lib/api/object-route.ts | 99 ++++++- src/lib/api/rate-limit.ts | 27 +- tests/api/db-objects.test.ts | 373 ++++++++++++++++++++++++- 4 files changed, 528 insertions(+), 15 deletions(-) create mode 100644 src/app/api/db/objects/source/route.ts diff --git a/src/app/api/db/objects/source/route.ts b/src/app/api/db/objects/source/route.ts new file mode 100644 index 00000000..7ef9dee2 --- /dev/null +++ b/src/app/api/db/objects/source/route.ts @@ -0,0 +1,44 @@ +import { NextRequest } from "next/server"; +import { + boundSourceDocument, + handleObjectRequest, + requireObjectPath, + requireSourceReader, + requireString, +} from "@/lib/api/object-route"; +import { SOURCE_CHARACTER_LIMIT } from "@/lib/db/object-kinds"; + +export const dynamic = "force-dynamic"; + +/** + * One object's definition text, as a document of named parts (#789 Phase 2). + * + * The seventh route under this prefix and the first one that can answer nothing: every other + * object method is required on `DatabaseProvider`, while `readObjectSource` is optional because + * two engines in the fleet hold no kind with a definition text anywhere. `requireSourceReader` is + * where that gap becomes a refusal a caller can read, in one branch of two conjuncts. + * + * `kind` is required in the body, not optional and not inferred, for the reason + * `describe/route.ts` gives and one more the research measured: on MySQL, MariaDB and DuckDB one + * name addresses more than one object of different kinds in one container, so a path alone reads + * the wrong object. + * + * No depth check, for the reason `describe/route.ts` gives: this is an object path, not a + * container path, and how deep a kind nests is a per-kind fact the provider's declaration carries. + * Two engines have a kind at mixed depth. + * + * `limit` is NOT accepted from the caller in Phase 2 and the route always passes + * `SOURCE_CHARACTER_LIMIT`. The argument exists on the provider method because that is where a + * bound belongs and because the conformance helper drives the bounded arm with a small number. An + * unused request field would be a second way to reach one behaviour. The route then applies the + * same bound to the ANSWER rather than trusting it: a number passed to an implementation outside + * our compiler, which the embedded seam's host is, is a request and not a bound. + */ +export async function POST(req: NextRequest) { + return handleObjectRequest(req, "api/db/objects/source", async (provider, body) => { + const path = requireObjectPath(body); + const kind = requireString(body, "kind"); + const read = requireSourceReader(provider, kind); + return boundSourceDocument(await read(path, kind, SOURCE_CHARACTER_LIMIT), SOURCE_CHARACTER_LIMIT); + }); +} diff --git a/src/lib/api/object-route.ts b/src/lib/api/object-route.ts index b8d9565c..0ed07c8b 100644 --- a/src/lib/api/object-route.ts +++ b/src/lib/api/object-route.ts @@ -3,7 +3,15 @@ import { getOrCreateProvider } from "@/lib/db"; import { createErrorResponse } from "@/lib/api/errors"; import { resolveConnection } from "@/lib/seed/resolve-connection"; import { guardRoute } from "@/lib/api/require-session"; -import { containerDepth, declaredKinds, findKind } from "@/lib/db/object-kinds"; +import { + SOURCE_PART_LIMIT, + containerDepth, + declaredKinds, + findKind, + isSourcePartUnavailable, + kindHasSource, + sourceBoundTruncationReason, +} from "@/lib/db/object-kinds"; import { INVENTORY_LIMIT, INVENTORY_PAIR_LIMIT, PAIR_TRUNCATION_REASON } from "@/lib/db/inventory-bounds"; import type { DatabaseConnection, @@ -11,14 +19,18 @@ import type { DatabaseProvider, ObjectDetail, ObjectKindSpec, + ObjectSourceDocument, + ObjectSourcePart, } from "@/lib/db/types"; /** - * Shared request handling for the six object-tree routes under /api/db/objects (#789). + * Shared request handling for the seven object routes under /api/db/objects (#789). * - * One handler rather than six copies, on the precedent of `src/lib/api/schema-route.ts`: the - * guard-then-parse ordering below is a security property, and six copies of it would be six - * chances for one of them to drift back to parsing first. + * One handler rather than seven copies: the guard-then-parse ordering below is a security + * property, and seven copies of it would be seven chances for one of them to drift back to + * parsing first. The seventh, the source read, was built on this handler rather than beside it + * and inherited auth-before-parse, rate limiting, connection resolution and error mapping with no + * new line of any of them. * * `route` is the same string the caller passes for error-response context, so `POST /${route}` * reuses it rather than threading a second, guard-specific string through every call site. @@ -209,6 +221,83 @@ export function resolveKinds(provider: DatabaseProvider, requested?: readonly st }); } +/** + * The provider's source reader for one kind, or a 400 saying it has none (#789 Phase 2). + * + * ONE branch with TWO conjuncts, on purpose. The first is reachable on every engine: a kind that + * declares no `hasSource` is an ordinary thing to ask for, because a caller can hold a stale menu + * or a path it built itself. The second is reachable through a provider that declares the kind and + * omits the method, which `readObjectSource` being optional makes representable and only this + * check makes visible. Folding them into two `if`s would give the second one a line whose only + * purpose is a state the first already excluded, which is the shape the deleted 501 arm had. + * + * It is a helper here rather than a `throw` in the route for the reason every 400 in this module + * is: `ObjectRouteError` is module-private, and keeping it private is what keeps the status + * vocabulary in one file rather than letting each route mint its own. + * + * 400 and not 404 or 501, following `resolveKinds`: answering nothing reads as a claim about the + * DATA when the truth is a claim about the ENGINE. + * + * The returned function is BOUND to the provider, because it is read off the instance as a value + * and a provider method that reaches its own pool through `this` would otherwise be called with + * no receiver. + */ +export function requireSourceReader( + provider: DatabaseProvider, + kind: string, +): (path: readonly string[], kind: string, limit?: number) => Promise { + const read = provider.readObjectSource; + if (!kindHasSource(provider.getCapabilities(), kind) || read === undefined) { + throw new ObjectRouteError(`${provider.type} declares no readable source for kind "${kind}"`, 400); + } + return read.bind(provider); +} + +/** + * The answered document under the route's OWN bound (#789 Phase 2). + * + * The route ENFORCES rather than trusts, which is the shipped precedent and not a new rule: the + * inventory route applies its own two bounds on top of the bound it hands `describeObjects`. Here + * there are sixteen providers plus any host implementing the embedded source seam, and the route + * materialises the whole answer and serialises it in one `NextResponse.json`, so this is the one + * place a memory bound can actually be held. A number merely PASSED to an implementation outside + * our compiler is a request, not a bound. + * + * A provider that bounded correctly is returned unchanged, which is what makes the walk safe to + * run on every answer. A provider that bounded at its own SMALLER limit is also unchanged, because + * its text already fits. Only a provider that over-answered is sliced, and its own sentence is + * KEPT and joined rather than replaced: a second bound is a second fact. + * + * `parts.length` is bounded too, because the tuple type has no upper bound and the real response + * size is `limit` times the part count. `SOURCE_PART_LIMIT` is four times the largest shape any + * engine in the fleet produces, so no correct provider can reach it and a host that does is a + * caller mistake rather than a database fact. + */ +export function boundSourceDocument(document: ObjectSourceDocument, limit: number): ObjectSourceDocument { + if (document.parts.length > SOURCE_PART_LIMIT) { + throw new ObjectRouteError( + `the source read answered ${document.parts.length} parts and this route carries at most ${SOURCE_PART_LIMIT}`, + 400, + ); + } + // Destructured rather than mapped, because `parts` is a NON-EMPTY tuple and `Array.prototype.map` + // answers a plain array that no longer satisfies it. + const [first, ...rest] = document.parts; + return { ...document, parts: [boundPart(first, limit), ...rest.map((part) => boundPart(part, limit))] }; +} + +function boundPart(part: ObjectSourcePart, limit: number): ObjectSourcePart { + // A refusal carries no text, so there is nothing to bound and nothing to mark. Reading `.text` + // on one would be a property access on the arm that does not declare it. + if (isSourcePartUnavailable(part) || part.text.length <= limit) return part; + const reason = sourceBoundTruncationReason(limit); + return { + ...part, + text: part.text.slice(0, limit), + truncated: { limit, reason: part.truncated === undefined ? reason : `${part.truncated.reason}; ${reason}` }, + }; +} + // The four inventory bounds are `src/lib/db/inventory-bounds.ts`'s, and they are re-exported // here because this route and the agent's grounding walk have to bound one read the same way. // They were declared in both modules until Task 28a gave them one owner (#789). diff --git a/src/lib/api/rate-limit.ts b/src/lib/api/rate-limit.ts index bed18a43..51804d63 100644 --- a/src/lib/api/rate-limit.ts +++ b/src/lib/api/rate-limit.ts @@ -119,14 +119,25 @@ const BUCKETS: Record = { // many model calls of its own, so this bounds how often LLM work is STARTED, never how much it // spends. ai: { maxVar: "RATE_LIMIT_AI_MAX", windowVar: "RATE_LIMIT_AI_WINDOW_SEC", maxDefault: 20, windowDefault: 60 }, - // Shared across every route that reaches a database - query, multi-query, transaction, - // disconnect, cancel, health, maintenance, monitoring, pool-stats, profile, provider-meta, - // schema, schema/list, schema/relations, schema-snapshot, test-connection, admin/fleet-health, - // plus the three storage routes (storage, storage/[collection], storage/migrate): TWENTY routes - // today (grep -rl 'bucket: "query"' src/app/api/ finds eighteen; schema/list and schema/relations - // reach this bucket indirectly, through schema-route.ts's shared handleSchemaRequest). The same - // workload reached through a different endpoint must not get a second budget - re-verify and - // correct this comment again if guardRoute grows a new call site. + // Shared across every route that reaches a database. RE-MEASURED 2026-09-12 (#789 Phase 2), + // because the previous count was stale in both directions: it said TWENTY and named four schema + // routes that no longer exist. `src/lib/api/schema-route.ts`, `db/schema`, `db/schema/list`, + // `db/schema/relations` and `db/schema-snapshot` were all removed with the object surface, and + // the object routes it never mentioned had joined. + // + // TWENTY-THREE handlers today, and there are two ways in, which is why one grep under-counts. + // Directly, sixteen call sites that pass bucket: "query" to guardRoute themselves + // (grep -rl 'bucket: "query"' src/app/api/ finds all sixteen): admin/fleet-health, db/cancel, + // db/disconnect, db/health, db/maintenance, db/monitoring, db/multi-query, db/pool-stats, + // db/profile, db/provider-meta, db/query, db/test-connection, db/transaction, and the three + // storage routes (storage, storage/[collection], storage/migrate). Note db/health: only its POST + // is metered, because the GET is the container health probe and takes no connection. + // Indirectly, the SEVEN object routes under db/objects (containers, counts, list, describe, + // search, inventory, source), which reach this bucket through handleObjectRequest in + // object-route.ts and so carry no bucket literal of their own. + // + // The same workload reached through a different endpoint must not get a second budget - + // re-verify and correct this comment again if guardRoute grows a new call site. // // The storage family joined when AU1 moved it onto the shared 401 (2026-08-22), and that gave it // a limiter it never had. It belongs here rather than in a bucket of its own: under diff --git a/tests/api/db-objects.test.ts b/tests/api/db-objects.test.ts index 9ac08f03..2fc3d57a 100644 --- a/tests/api/db-objects.test.ts +++ b/tests/api/db-objects.test.ts @@ -3,6 +3,7 @@ import { createMockRequest, parseResponseJSON } from "../helpers/mock-next"; import { createMockProvider } from "../helpers/mock-provider"; import { clearRateLimitState } from "@/lib/api/rate-limit"; import { INVENTORY_LIMIT, INVENTORY_PAIR_LIMIT } from "@/lib/api/object-route"; +import { SOURCE_CHARACTER_LIMIT, SOURCE_PART_LIMIT, sourceBoundTruncationReason } from "@/lib/db/object-kinds"; import { ApiErrorCode } from "@/lib/api/error-codes"; import { QueryError } from "@/lib/db/errors"; import type { @@ -14,6 +15,8 @@ import type { KindCount, ObjectDetail, ObjectKindSpec, + ObjectSourceDocument, + ObjectSourcePart, } from "@/lib/db/types"; import { DatabaseError, @@ -100,6 +103,7 @@ const listRoute = await import("@/app/api/db/objects/list/route"); const describeRoute = await import("@/app/api/db/objects/describe/route"); const searchRoute = await import("@/app/api/db/objects/search/route"); const inventoryRoute = await import("@/app/api/db/objects/inventory/route"); +const sourceRoute = await import("@/app/api/db/objects/source/route"); // ============================================================================ // Fixtures @@ -122,6 +126,7 @@ interface ProviderShape { listObjects?: DatabaseProvider["listObjects"]; describeObject?: DatabaseProvider["describeObject"]; describeObjects?: DatabaseProvider["describeObjects"]; + readObjectSource?: DatabaseProvider["readObjectSource"]; } /** A provider that declares one schema level and two kinds unless the test says otherwise. */ @@ -138,6 +143,7 @@ function objectProvider(shape: ProviderShape = {}): DatabaseProvider { if (shape.listObjects) provider.listObjects = shape.listObjects; if (shape.describeObject) provider.describeObject = shape.describeObject; if (shape.describeObjects) provider.describeObjects = shape.describeObjects; + if (shape.readObjectSource) provider.readObjectSource = shape.readObjectSource; return provider; } @@ -178,8 +184,8 @@ describe("the shared guard", () => { expect(mockGetOrCreateProvider).toHaveBeenCalledTimes(0); }); - test("every one of the six routes refuses an unauthenticated caller", async () => { - const routes = [containersRoute, countsRoute, listRoute, describeRoute, searchRoute, inventoryRoute]; + test("every one of the seven routes refuses an unauthenticated caller", async () => { + const routes = [containersRoute, countsRoute, listRoute, describeRoute, searchRoute, inventoryRoute, sourceRoute]; for (const route of routes) { mockGetSession.mockResolvedValueOnce(null as unknown as { role: string; username: string }); const response = await route.POST( @@ -1202,3 +1208,366 @@ describe("POST /api/db/objects/inventory", () => { expect("defaultContainer" in body).toBe(false); }); }); + +// ============================================================================ +// source +// ============================================================================ + +describe("POST /api/db/objects/source", () => { + const FUNCTION_KIND: ObjectKindSpec = { + id: "function", + role: "routine", + label: "Function", + labelPlural: "Functions", + hasSource: true, + sourceLanguage: "sql", + }; + + function readablePart(overrides: Partial> = {}): ObjectSourcePart { + return { + id: "body", + label: "Body", + text: "CREATE FUNCTION order_total(integer) RETURNS integer AS $$ SELECT 1 $$ LANGUAGE sql", + language: "sql", + form: "complete", + origin: "regenerated", + ...overrides, + }; + } + + /** + * A provider that answers only the kind it declares and raises its OWN error for anything else. + * + * The raise is what makes the gate's two conjuncts distinguishable. A double that answered any + * kind would turn the undeclared-kind test into a 200-versus-400 comparison, and deleting the + * declaration conjunct would then be caught by the status alone. Raising a `QueryError` is what + * a real provider does when asked for an object it cannot read, and `createErrorResponse` maps + * it to the same 400 the gate uses, so only the SENTENCE separates the two. + */ + function sourceProviderReading(document: ObjectSourceDocument): DatabaseProvider { + return objectProvider({ + objectKinds: [TABLE_KIND, FUNCTION_KIND], + readObjectSource: mock(async (path: readonly string[], kind: string) => { + if (kind !== "function") { + throw new QueryError(`the engine has no readable ${kind} at ${path.join(".")}`, "postgres"); + } + return document; + }), + }); + } + + function oneReadablePart(part: ObjectSourcePart = readablePart()): ObjectSourceDocument { + return { path: ["app", "order_total(integer)"], kind: "function", parts: [part] }; + } + + test("answers the provider's document for a source-bearing kind, bounded by the route", async () => { + const read = mock(async () => oneReadablePart()); + activeProvider = objectProvider({ objectKinds: [TABLE_KIND, FUNCTION_KIND], readObjectSource: read }); + + const response = await sourceRoute.POST( + createMockRequest("/api/db/objects/source", { + method: "POST", + body: { connection, path: ["app", "order_total(integer)"], kind: "function" }, + }) as never, + ); + + expect(response.status).toBe(200); + const body = await parseResponseJSON(response); + expect(body).toEqual(oneReadablePart()); + // The route names its own bound rather than leaving `limit` absent, which is what makes a + // provider that honours the argument bound the same way the route would have bounded it. + expect(read).toHaveBeenCalledWith(["app", "order_total(integer)"], "function", SOURCE_CHARACTER_LIMIT); + }); + + test("calls the reader with the provider as its receiver, so a method reading `this` still works", async () => { + // Every shipped provider reads `this` in this method: it is where the pool, the config and the + // escaper live. The reader is taken off the instance as a VALUE here, and a value called with + // no receiver has `this === undefined` under a module's strict mode, so the label below is + // read from the receiver rather than closed over. Unbound, this double raises a TypeError + // instead of answering, which is the difference a 200 and a label can see. + activeProvider = objectProvider({ + objectKinds: [FUNCTION_KIND], + readObjectSource: mock(async function (this: DatabaseProvider) { + return oneReadablePart(readablePart({ label: this.type })); + }), + }); + + const response = await sourceRoute.POST( + createMockRequest("/api/db/objects/source", { + method: "POST", + body: { connection, path: ["app", "f"], kind: "function" }, + }) as never, + ); + + expect(response.status).toBe(200); + const [part] = (await parseResponseJSON(response)).parts; + expect(part.label).toBe("postgres"); + }); + + test("refuses a kind that declares no source, naming the engine and the kind", async () => { + activeProvider = sourceProviderReading(oneReadablePart()); + + const response = await sourceRoute.POST( + createMockRequest("/api/db/objects/source", { + method: "POST", + body: { connection, path: ["app", "orders"], kind: "table" }, + }) as never, + ); + + expect(response.status).toBe(400); + // The SENTENCE and not the status: the double raises its own error for this kind, and that + // error also maps to 400, so a status-only assertion survives deleting the declaration + // conjunct of the gate. + expect((await parseResponseJSON<{ error: string }>(response)).error).toBe( + 'postgres declares no readable source for kind "table"', + ); + }); + + test("refuses a provider that declares hasSource and implements no method, with the same sentence", async () => { + activeProvider = objectProvider({ objectKinds: [TABLE_KIND, FUNCTION_KIND] }); + + const response = await sourceRoute.POST( + createMockRequest("/api/db/objects/source", { + method: "POST", + body: { connection, path: ["app", "f"], kind: "function" }, + }) as never, + ); + + expect(response.status).toBe(400); + expect((await parseResponseJSON<{ error: string }>(response)).error).toBe( + 'postgres declares no readable source for kind "function"', + ); + }); + + test("refuses a kind the engine does not declare at all", async () => { + activeProvider = sourceProviderReading(oneReadablePart()); + + const response = await sourceRoute.POST( + createMockRequest("/api/db/objects/source", { + method: "POST", + body: { connection, path: ["app", "f"], kind: "procedure" }, + }) as never, + ); + + expect(response.status).toBe(400); + expect((await parseResponseJSON<{ error: string }>(response)).error).toBe( + 'postgres declares no readable source for kind "procedure"', + ); + }); + + test("refuses an empty path", async () => { + activeProvider = sourceProviderReading(oneReadablePart()); + + const response = await sourceRoute.POST( + createMockRequest("/api/db/objects/source", { + method: "POST", + body: { connection, path: [], kind: "function" }, + }) as never, + ); + + expect(response.status).toBe(400); + expect((await parseResponseJSON<{ error: string }>(response)).error).toContain("must name an object"); + }); + + test("refuses a missing kind", async () => { + activeProvider = sourceProviderReading(oneReadablePart()); + + const response = await sourceRoute.POST( + createMockRequest("/api/db/objects/source", { + method: "POST", + body: { connection, path: ["app", "f"] }, + }) as never, + ); + + expect(response.status).toBe(400); + expect((await parseResponseJSON<{ error: string }>(response)).error).toContain('"kind" must be a non-empty string'); + }); + + test("bounds a provider that ignores the limit, and marks what it bounded", async () => { + activeProvider = sourceProviderReading( + oneReadablePart(readablePart({ text: "x".repeat(SOURCE_CHARACTER_LIMIT + 10) })), + ); + + const response = await sourceRoute.POST( + createMockRequest("/api/db/objects/source", { + method: "POST", + body: { connection, path: ["app", "f"], kind: "function" }, + }) as never, + ); + + expect(response.status).toBe(200); + const body = await parseResponseJSON(response); + const [part] = body.parts; + if ("unavailable" in part) throw new Error("the double answers a readable part"); + expect(part.text).toHaveLength(SOURCE_CHARACTER_LIMIT); + expect(part.truncated).toEqual({ + limit: SOURCE_CHARACTER_LIMIT, + reason: sourceBoundTruncationReason(SOURCE_CHARACTER_LIMIT), + }); + }); + + test("joins its own sentence to a bound the provider already reported", async () => { + activeProvider = sourceProviderReading( + oneReadablePart( + readablePart({ + text: "y".repeat(SOURCE_CHARACTER_LIMIT + 10), + truncated: { limit: 4000, reason: "the engine stopped at 4,000 characters" }, + }), + ), + ); + + const response = await sourceRoute.POST( + createMockRequest("/api/db/objects/source", { + method: "POST", + body: { connection, path: ["app", "f"], kind: "function" }, + }) as never, + ); + + const body = await parseResponseJSON(response); + const [part] = body.parts; + if ("unavailable" in part) throw new Error("the double answers a readable part"); + // Two bounds are two facts, so the engine's own sentence is kept beside the route's rather + // than replaced by it. + expect(part.truncated?.reason).toBe( + `the engine stopped at 4,000 characters; ${sourceBoundTruncationReason(SOURCE_CHARACTER_LIMIT)}`, + ); + expect(part.truncated?.limit).toBe(SOURCE_CHARACTER_LIMIT); + }); + + test("leaves a part that already fits exactly as the provider wrote it", async () => { + const exact = readablePart({ text: "z".repeat(SOURCE_CHARACTER_LIMIT) }); + activeProvider = sourceProviderReading(oneReadablePart(exact)); + + const response = await sourceRoute.POST( + createMockRequest("/api/db/objects/source", { + method: "POST", + body: { connection, path: ["app", "f"], kind: "function" }, + }) as never, + ); + + const body = await parseResponseJSON(response); + const [part] = body.parts; + if ("unavailable" in part) throw new Error("the double answers a readable part"); + // An exact answer is never marked: marking one teaches a reader to discount every mark. + expect(part.truncated).toBeUndefined(); + expect(part.text).toHaveLength(SOURCE_CHARACTER_LIMIT); + }); + + test("bounds a part that is not the first one, so the walk is not a first-part special case", async () => { + const document: ObjectSourceDocument = { + path: ["app", "pkg"], + kind: "function", + parts: [ + readablePart({ id: "spec", label: "Specification", text: "SHORT" }), + readablePart({ id: "body", label: "Body", text: "w".repeat(SOURCE_CHARACTER_LIMIT + 1) }), + ], + }; + activeProvider = sourceProviderReading(document); + + const response = await sourceRoute.POST( + createMockRequest("/api/db/objects/source", { + method: "POST", + body: { connection, path: ["app", "pkg"], kind: "function" }, + }) as never, + ); + + const body = await parseResponseJSON(response); + const [spec, second] = body.parts; + if ("unavailable" in spec || second === undefined || "unavailable" in second) { + throw new Error("the double answers two readable parts"); + } + expect(spec.truncated).toBeUndefined(); + expect(second.text).toHaveLength(SOURCE_CHARACTER_LIMIT); + expect(second.truncated?.reason).toBe(sourceBoundTruncationReason(SOURCE_CHARACTER_LIMIT)); + }); + + test("carries a refused part through untouched, because a refusal has no text to bound", async () => { + const refusal: ObjectSourcePart = { + id: "body", + label: "Body", + unavailable: "u".repeat(SOURCE_CHARACTER_LIMIT + 10), + }; + activeProvider = sourceProviderReading(oneReadablePart(refusal)); + + const response = await sourceRoute.POST( + createMockRequest("/api/db/objects/source", { + method: "POST", + body: { connection, path: ["app", "f"], kind: "function" }, + }) as never, + ); + + expect(response.status).toBe(200); + const body = await parseResponseJSON(response); + const [part] = body.parts; + if (!("unavailable" in part)) throw new Error("the double answers a refused part"); + expect(part.unavailable).toHaveLength(SOURCE_CHARACTER_LIMIT + 10); + }); + + test("refuses a document carrying more parts than the route will carry", async () => { + const parts = Array.from({ length: SOURCE_PART_LIMIT + 1 }, (_unused, index) => + readablePart({ id: `p${index}`, label: `Part ${index}` }), + ) as unknown as ObjectSourceDocument["parts"]; + activeProvider = sourceProviderReading({ path: ["app", "f"], kind: "function", parts }); + + const response = await sourceRoute.POST( + createMockRequest("/api/db/objects/source", { + method: "POST", + body: { connection, path: ["app", "f"], kind: "function" }, + }) as never, + ); + + expect(response.status).toBe(400); + expect((await parseResponseJSON<{ error: string }>(response)).error).toBe( + `the source read answered ${SOURCE_PART_LIMIT + 1} parts and this route carries at most ${SOURCE_PART_LIMIT}`, + ); + }); + + test("carries a document holding exactly the part limit", async () => { + const parts = Array.from({ length: SOURCE_PART_LIMIT }, (_unused, index) => + readablePart({ id: `p${index}`, label: `Part ${index}`, text: "ok" }), + ) as unknown as ObjectSourceDocument["parts"]; + activeProvider = sourceProviderReading({ path: ["app", "f"], kind: "function", parts }); + + const response = await sourceRoute.POST( + createMockRequest("/api/db/objects/source", { + method: "POST", + body: { connection, path: ["app", "f"], kind: "function" }, + }) as never, + ); + + expect(response.status).toBe(200); + expect((await parseResponseJSON(response)).parts).toHaveLength(SOURCE_PART_LIMIT); + }); + + test("carries the engine's own error to a 400 rather than inventing one", async () => { + activeProvider = objectProvider({ + objectKinds: [FUNCTION_KIND], + readObjectSource: mock(async () => { + throw new QueryError("permission denied for schema app", "postgres"); + }), + }); + + const response = await sourceRoute.POST( + createMockRequest("/api/db/objects/source", { + method: "POST", + body: { connection, path: ["app", "f"], kind: "function" }, + }) as never, + ); + + expect(response.status).toBe(400); + const body = await parseResponseJSON<{ error: string; code: string }>(response); + expect(body.error).toBe("permission denied for schema app"); + expect(body.code).toBe(ApiErrorCode.QUERY_ERROR); + }); + + test("refuses an unauthenticated caller before parsing a body", async () => { + mockGetSession.mockResolvedValueOnce(null as unknown as { role: string; username: string }); + + const response = await sourceRoute.POST( + new Request("http://localhost:3000/api/db/objects/source", { method: "POST" }) as never, + ); + + expect(response.status).toBe(401); + expect(mockGetOrCreateProvider).toHaveBeenCalledTimes(0); + }); +}); From b19ad65bddf0a5bd768d43cae22fa9a6bc92e185 Mon Sep 17 00:00:00 2001 From: cevheri Date: Sat, 12 Sep 2026 22:36:48 +0300 Subject: [PATCH 11/94] feat(ui): add the read-only object source viewer (#789) The one rule this surface exists for: an unreadable source never opens an empty editor. An empty editor reads as "there is no source", and a user who types over it deletes the object, which is the composition DBeaver ships, measured in its source. It is closed twice here. The type gives a refused part no text key, and the component renders a DIFFERENT element for a refusal and for a failed read, so there is no editor on screen to type into even if a later change made one writable. The renderer does not lean on the type for this, because the union does not make a part carrying both keys a compile error: the excess-property check on a union admits a property declared on any member, so isSourceDocumentShape refuses that part first. Not QueryEditor, for four measured reasons: it hardcodes readOnly false with no prop to change it, installs a Run action and a Cmd+Enter binding unconditionally on mount, renders an execute toolbar, and takes a closed four-member language union reachable only through resolveTabType, which CLAUDE.md forbids extending. A definition opened in it would offer to execute itself. readOnly true is not a security boundary either and the docblock says so; the boundary in Phase 2 is that no write path exists at all. The whole read lives in one place so the two shells cannot drift, keyed on the address rather than on the array and object props, which are a fresh identity every render. The token recorded is the one at the moment the read was ISSUED: a DDL that runs while a read is in flight cannot be attributed to either side of it, so the tab marks itself stale and offers a re-read rather than claiming the text is current. The barrel re-exports six names and not the caption, which has no consumer outside this folder; object-tree/index.ts records what happens otherwise. Also registers the suite as its own group in tests/run-components.sh. That file has no named owner in the phase's ownership map, and a component test file absent from it never runs in CI at all, which this repository has shipped once before. Group 27 is separate from Group 15 because that group holds QueryEditor.test.tsx, which installs a different double of @monaco-editor/react, and mock.module is process-wide. 18 mutations run and 18 killed. Raw lcov for the viewer: LF 172, LH 172, no DA record at zero. --- .../object-source/ObjectSourceView.tsx | 341 +++++++++++ src/components/object-source/index.ts | 16 + .../object-source/ObjectSourceView.test.tsx | 572 ++++++++++++++++++ tests/run-components.sh | 10 +- 4 files changed, 938 insertions(+), 1 deletion(-) create mode 100644 src/components/object-source/ObjectSourceView.tsx create mode 100644 src/components/object-source/index.ts create mode 100644 tests/components/object-source/ObjectSourceView.test.tsx diff --git a/src/components/object-source/ObjectSourceView.tsx b/src/components/object-source/ObjectSourceView.tsx new file mode 100644 index 00000000..e79eff76 --- /dev/null +++ b/src/components/object-source/ObjectSourceView.tsx @@ -0,0 +1,341 @@ +"use client"; + +import Editor from "@monaco-editor/react"; +import { FileWarning, LoaderCircle, RefreshCw, TriangleAlert } from "lucide-react"; +import React, { useCallback, useEffect, useId, useMemo, useRef } from "react"; +import { httpSourceReader, isSourceDocumentShape, type ObjectSourceReader } from "./source-reader"; +import { sourceCaption } from "./source-caption"; +import { Button } from "@/components/ui/button"; +import { isSourcePartUnavailable } from "@/lib/db/object-kinds"; +import { pathKey } from "@/lib/db/object-path"; +import type { ObjectSourceDocument, ObjectSourcePart } from "@/lib/db/types"; +import { configureMonacoLoader } from "@/lib/editor/monaco-loader"; +import { defineStudioThemes, STUDIO_THEME_DARK, STUDIO_THEME_LIGHT } from "@/lib/editor/monaco-theme"; +import { useEffectiveTheme } from "@/hooks/use-effective-theme"; +import type { DatabaseConnection } from "@/lib/types"; + +// Serve Monaco from our own origin rather than @monaco-editor/react's jsdelivr default. +// Called at module scope HERE as well as in `QueryEditor`, because it must run before the +// FIRST mount and a Source tab can be the first editor a session opens: a restored tab set +// whose active tab is a Source tab paints this component with no query editor ever mounted. +// `loader.config` is idempotent, so the second call rewrites the same path with the same value. +configureMonacoLoader(); + +/** + * What a shell hands back to the tab when this viewer learns something (#789). + * + * The whole read lives in ONE place, this component, and the result is written back through + * `onChange` so the tab keeps it across a tab switch and an unmount. Every field is optional + * and a shell MERGES BY SPREAD, which is what makes an explicitly-`undefined` field a CLEAR: + * the stale banner's control sends `{ document: undefined, failure: undefined, readAtToken: + * undefined }` and the three keys are present on purpose, because an omitted key would leave + * the stale document in place and the re-read would never be issued. + */ +export interface ObjectSourcePatch { + readonly document?: ObjectSourceDocument; + readonly failure?: string; + readonly activePartId?: string; + readonly readAtToken?: number; +} + +export interface ObjectSourceViewProps { + readonly connection: DatabaseConnection; + readonly path: readonly string[]; + readonly kind: string; + /** The kind's own label from the declaration. The viewer never derives one from the id. */ + readonly kindLabel: string; + /** The object's display label. `DatabaseObject.name`, which is NOT the last path segment. */ + readonly displayName: string; + readonly document?: ObjectSourceDocument; + readonly failure?: string; + readonly activePartId?: string; + /** The session's catalog-change counter. The shell owns it; 0 where a shell has none. */ + readonly refreshToken: number; + /** The counter's value when this document was read. Absent until a read lands. */ + readonly readAtToken?: number; + /** Absent means the standalone route. The embedded shell passes the host's reader. */ + readonly reader?: ObjectSourceReader; + /** MUST be stable across renders, or the read effect re-issues for ever. */ + readonly onChange: (patch: ObjectSourcePatch) => void; +} + +/** The sentence for a body neither shell can draw, which is OUR fact and not the engine's. */ +const UNRENDERABLE = "The source read answered with a body this viewer cannot render."; + +/** + * The active part, and the fallback that makes the switcher's selection total. + * + * `activePartId` is remembered on the tab and the document is re-read from the engine, so the + * two can disagree: a provider that renames a part between two reads, or a restored tab whose + * remembered id belonged to an earlier shape. Falling back to the first part is what stops that + * disagreement rendering as nothing at all, which is the empty-versus-unreadable collapse this + * whole surface exists to prevent, one level in. + * + * The first part is addressed as a construction and never as a positional read of a path: + * `parts` is a non-empty tuple, so `parts[0]` is total by the type. + */ +function activePart(document: ObjectSourceDocument, activePartId: string | undefined): ObjectSourcePart { + return document.parts.find((part) => part.id === activePartId) ?? document.parts[0]; +} + +/** + * The read-only viewer for one object's definition (#789). + * + * NOT `QueryEditor`, and the four reasons are measured rather than stylistic: that component + * hardcodes `readOnly: false` with no prop to change it, installs a Run action and a Cmd+Enter + * binding unconditionally on mount, renders an execute toolbar, and takes a closed four-member + * `language` union reachable only through `resolveTabType`, which `CLAUDE.md` forbids + * extending. A definition opened in it would offer to EXECUTE itself. + * + * `readOnly: true` is NOT a security boundary and this component does not pretend otherwise. + * MEASURED on `@monaco-editor/react` 4.7.0: it blocks USER edits only, and the `value` effect + * still calls `setValue` programmatically, so anything holding the editor handle can write to + * the model. The boundary in Phase 2 is that no write path exists at all: no Run action is + * installed, no key binding is added, and nothing reachable from here can execute a statement. + * + * THE ONE RULE THIS SURFACE EXISTS FOR: an unreadable source never opens an empty editor. An + * empty editor reads as "there is no source", and a user who types over it deletes the object, + * which is measured in DBeaver's own source. It is closed twice here. The TYPE gives a refused + * part no `text` key. The COMPONENT renders a DIFFERENT element for a refusal and for a failed + * read, so there is no editor on screen to type into even if a later change made one writable. + * The renderer does not rely on the type for this, because the union does NOT make a part + * carrying both `text` and `unavailable` a compile error: TypeScript's excess-property check on + * a union admits any property declared on any member, so such a part narrows to the refusal. + * `isSourceDocumentShape` refuses that part before it can reach this function. + * + * Nothing rendered here reads a kind id or a database type id. The kind's label arrives as a + * prop from the declaration, the part's label is the engine's own word, and the language + * travels on the part. + */ +export function ObjectSourceView(props: ObjectSourceViewProps): React.JSX.Element { + const { connection, path, kind, document: sourceDocument, failure, refreshToken, reader, onChange } = props; + const theme = useEffectiveTheme(); + const baseId = useId(); + + /** + * The read's identity, and the reason it is a STRING rather than the props themselves. + * + * `path` is an array prop and `connection` is an object prop, so both are a fresh identity on + * every render of the shell. An effect keyed on either re-runs on every render, and an effect + * that cancels its in-flight read in a cleanup would then cancel it for ever and the document + * would never land. Keying on the address, plus a ref recording the address already asked, + * means a re-render with identical props issues nothing and a genuinely new object issues one. + * + * `pathKey` and never `JSON.stringify(path)`, per standing ruling 5g: the key separator is a + * control character no engine admits inside an identifier, so `["a.b"]` and `["a", "b"]` + * cannot collide, while JSON escaping rewrites exotic names. + */ + const address = `${connection.id}/${pathKey(path)}/${kind}`; + const asked = useRef(undefined); + const needsRead = sourceDocument === undefined && failure === undefined; + + useEffect(() => { + if (!needsRead) { + asked.current = undefined; + return; + } + if (asked.current === address) return; + asked.current = address; + /* + * The counter's value AT THE MOMENT THE READ WAS ISSUED, not when it landed. A DDL that + * runs while this read is in flight cannot be attributed to either side of it, so recording + * the earlier value marks the tab stale and offers a re-read, which is the honest half of + * the repository's absence grammar: the client knows a DDL ran and does not know whether + * this object changed. + */ + const tokenAtRead = refreshToken; + /* + * No cleanup and no mounted guard, deliberately. An answer is dropped only when the + * ADDRESS has moved on, which the ref records. A viewer unmounted by a tab switch still + * writes its answer through `onChange`, and that is wanted rather than tolerated: the + * patch lands on the tab's own state, so the read a user started before switching away is + * there when they switch back instead of being issued a second time. + */ + void (reader ?? httpSourceReader)(connection, path, kind).then( + (answer) => { + if (asked.current !== address) return; + if (!isSourceDocumentShape(answer)) { + onChange({ failure: UNRENDERABLE, readAtToken: tokenAtRead }); + return; + } + onChange({ document: answer, activePartId: answer.parts[0].id, readAtToken: tokenAtRead }); + }, + (error: unknown) => { + if (asked.current !== address) return; + onChange({ + failure: error instanceof Error ? error.message : String(error), + readAtToken: tokenAtRead, + }); + }, + ); + // `path` and `connection` are read through `address`; `reader` and `onChange` are documented + // as stable, and a change in either is answered by the address guard rather than a re-issue. + }, [address, needsRead, refreshToken, connection, path, kind, reader, onChange]); + + const reread = useCallback(() => { + onChange({ document: undefined, failure: undefined, readAtToken: undefined }); + }, [onChange]); + + const part = useMemo( + () => (sourceDocument === undefined ? undefined : activePart(sourceDocument, props.activePartId)), + [sourceDocument, props.activePartId], + ); + + /* + * A read lands with `readAtToken` set, so an absent one is "nothing has been read yet" and + * never "read at token zero". Zero is a real token: it is what every shell that counts no + * DDL passes for the whole session. + */ + const stale = props.readAtToken !== undefined && props.readAtToken !== refreshToken; + const parts = sourceDocument?.parts ?? []; + const showSwitcher = parts.length > 1; + const tabId = (index: number) => `${baseId}-tab-${index}`; + const panelId = (index: number) => `${baseId}-panel-${index}`; + const activeIndex = parts.findIndex((candidate) => candidate === part); + + return ( +
+
+ + {props.displayName} + + + {props.kindLabel} + +
+ + {stale && ( +
+
+ )} + + {failure !== undefined ? ( +
+
+ ) : part === undefined ? ( +
+
+ ) : ( + <> + {showSwitcher && ( +
+ {parts.map((candidate, index) => ( + + ))} +
+ )} +
+ {isSourcePartUnavailable(part) ? ( + /* + * A DIFFERENT component, not an editor with an empty buffer. This is the + * composition DBeaver gets wrong, measured in its source: an unreadable + * definition reaches a writable editor holding one comment line. + */ +
+
+ ) : ( + <> +

+ {sourceCaption(part.form, part.origin)} +

+ {part.truncated !== undefined && ( +
+
+ )} +
+ +
+ + )} +
+ + )} +
+ ); +} diff --git a/src/components/object-source/index.ts b/src/components/object-source/index.ts new file mode 100644 index 00000000..c93679f3 --- /dev/null +++ b/src/components/object-source/index.ts @@ -0,0 +1,16 @@ +/** + * What a SHELL imports from the object source package, and nothing else. + * + * The folder's own modules import each other by path and the tests import the unit under test + * by path, so a re-export here earns its place only by having a consumer OUTSIDE this + * directory: both shells mount `ObjectSourceView`, declare their tab state with + * `ObjectSourcePatch`, and the embedded shell builds an `ObjectSourceReader` from its host's + * method while the standalone one falls through to `httpSourceReader`. + * + * `sourceCaption` is deliberately absent. It is imported by path inside this folder and has no + * consumer outside it, and `object-tree/index.ts` records what happens otherwise: the required + * `knip` check named eighteen re-exported lines there as reaching nobody, and a barrel that + * re-exports everything cannot be read as a statement about what the outside uses (#789). + */ +export { ObjectSourceView, type ObjectSourcePatch, type ObjectSourceViewProps } from "./ObjectSourceView"; +export { httpSourceReader, isSourceDocumentShape, type ObjectSourceReader } from "./source-reader"; diff --git a/tests/components/object-source/ObjectSourceView.test.tsx b/tests/components/object-source/ObjectSourceView.test.tsx new file mode 100644 index 00000000..b369cddb --- /dev/null +++ b/tests/components/object-source/ObjectSourceView.test.tsx @@ -0,0 +1,572 @@ +import "../../setup-dom"; +import "../../helpers/mock-navigation"; + +import { mock } from "bun:test"; +import React from "react"; + +/** + * The read-only object source viewer (#789). + * + * `@monaco-editor/react` is replaced with a `