From ab263b9b3719260d6fdaa03ac3b546c5e946847a Mon Sep 17 00:00:00 2001 From: IslaLiu Date: Tue, 21 Apr 2026 17:35:54 +0800 Subject: [PATCH 1/3] fix: surface source description in MCP tool description The `description` field on TOML [[sources]] was parsed into SourceConfig and exposed via the REST /api/sources endpoint, but never surfaced through MCP tool metadata. AI clients using DBHub via MCP only saw the hardcoded template "Execute SQL queries on the '' database" regardless of how the user described the source, making it impossible for models to route requests between multiple sources based on the user's descriptions. Prepend sourceConfig.description (when set) to both execute_sql and search_objects generated tool descriptions so AI clients see the source's purpose first while retaining all existing technical context (readonly note, max_rows note, dbType, source id). Custom tool descriptions (CustomToolConfig.description) are untouched because custom tools already carry their own required description. Fixes #267 --- .../tool-metadata.integration.test.ts | 74 +++++++++++++++++++ src/utils/tool-metadata.ts | 16 ++-- 2 files changed, 85 insertions(+), 5 deletions(-) create mode 100644 src/utils/__tests__/tool-metadata.integration.test.ts diff --git a/src/utils/__tests__/tool-metadata.integration.test.ts b/src/utils/__tests__/tool-metadata.integration.test.ts new file mode 100644 index 00000000..98eb7c67 --- /dev/null +++ b/src/utils/__tests__/tool-metadata.integration.test.ts @@ -0,0 +1,74 @@ +import { describe, it, expect, beforeAll, afterAll } from "vitest"; +import type { ConnectorManager } from "../../connectors/manager.js"; +import { setupManagerWithFixture, FIXTURES, loadFixtureConfig } from "../../__fixtures__/helpers.js"; +import { getExecuteSqlMetadata, getSearchObjectsMetadata } from "../tool-metadata.js"; +import { initializeToolRegistry } from "../../tools/registry.js"; + +// Import SQLite connector to ensure it's registered +import "../../connectors/sqlite/index.js"; + +describe("tool-metadata description propagation", () => { + let manager: ConnectorManager; + + beforeAll(async () => { + // readonly-maxrows fixture has three sources: + // readonly_limited: description = "Read-only database for safe queries", readonly + max_rows + // writable_limited: no description, writable + max_rows + // writable_unlimited: no description, writable + no limits + manager = await setupManagerWithFixture(FIXTURES.READONLY_MAXROWS); + const { sources, tools } = loadFixtureConfig(FIXTURES.READONLY_MAXROWS); + initializeToolRegistry({ sources, tools: tools || [] }); + }, 30000); + + afterAll(async () => { + if (manager) { + await manager.disconnect(); + } + }); + + describe("getExecuteSqlMetadata", () => { + it("prepends user description from source config when present", () => { + const metadata = getExecuteSqlMetadata("readonly_limited"); + + expect(metadata.description).toContain("Read-only database for safe queries"); + // Original technical context must still be preserved + expect(metadata.description).toContain("readonly_limited"); + expect(metadata.description).toContain("sqlite"); + expect(metadata.description).toContain("[READ-ONLY MODE]"); + // User description comes first, technical template follows + expect(metadata.description.indexOf("Read-only database for safe queries")).toBeLessThan( + metadata.description.indexOf("Execute SQL queries") + ); + }); + + it("omits the prefix entirely when source has no description", () => { + const metadata = getExecuteSqlMetadata("writable_limited"); + + // No extra prefix — description starts with the template + expect(metadata.description.startsWith("Execute SQL queries")).toBe(true); + expect(metadata.description).toContain("writable_limited"); + expect(metadata.description).toContain("sqlite"); + }); + }); + + describe("getSearchObjectsMetadata", () => { + it("prepends user description from source config when present", () => { + const metadata = getSearchObjectsMetadata("readonly_limited"); + + expect(metadata.description).toContain("Read-only database for safe queries"); + expect(metadata.description).toContain("readonly_limited"); + expect(metadata.description).toContain("sqlite"); + expect(metadata.description.indexOf("Read-only database for safe queries")).toBeLessThan( + metadata.description.indexOf("Search and list database objects") + ); + }); + + it("omits the prefix entirely when source has no description", () => { + const metadata = getSearchObjectsMetadata("writable_unlimited"); + + expect(metadata.description.startsWith("Search and list database objects")).toBe(true); + expect(metadata.description).toContain("writable_unlimited"); + expect(metadata.description).toContain("sqlite"); + }); + }); +}); diff --git a/src/utils/tool-metadata.ts b/src/utils/tool-metadata.ts index 710a6285..bfdf8528 100644 --- a/src/utils/tool-metadata.ts +++ b/src/utils/tool-metadata.ts @@ -106,12 +106,15 @@ export function getExecuteSqlMetadata(sourceId: string): ToolMetadata { ? `Execute SQL (${dbType})` : `Execute SQL on ${sourceId} (${dbType})`; - // Determine description with more context + // Determine description with more context. + // Prepend the user-provided `description` from the source config (if set) + // so AI clients reading the MCP tool list see the source's purpose first. + const userDescPrefix = sourceConfig.description ? `${sourceConfig.description}. ` : ""; const readonlyNote = executeOptions.readonly ? " [READ-ONLY MODE]" : ""; const maxRowsNote = executeOptions.maxRows ? ` (limited to ${executeOptions.maxRows} rows)` : ""; const description = isSingleSource - ? `Execute SQL queries on the ${dbType} database${readonlyNote}${maxRowsNote}` - : `Execute SQL queries on the '${sourceId}' ${dbType} database${readonlyNote}${maxRowsNote}`; + ? `${userDescPrefix}Execute SQL queries on the ${dbType} database${readonlyNote}${maxRowsNote}` + : `${userDescPrefix}Execute SQL queries on the '${sourceId}' ${dbType} database${readonlyNote}${maxRowsNote}`; // Build annotations object with all standard MCP hints const isReadonly = executeOptions.readonly === true; @@ -149,9 +152,12 @@ export function getSearchObjectsMetadata(sourceId: string): { name: string; desc const title = isSingleSource ? `Search Database Objects (${dbType})` : `Search Database Objects on ${sourceId} (${dbType})`; + // Prepend the user-provided `description` from the source config (if set) + // so AI clients reading the MCP tool list see the source's purpose first. + const userDescPrefix = sourceConfig.description ? `${sourceConfig.description}. ` : ""; const description = isSingleSource - ? `Search and list database objects (schemas, tables, columns, procedures, functions, indexes) on the ${dbType} database` - : `Search and list database objects (schemas, tables, columns, procedures, functions, indexes) on the '${sourceId}' ${dbType} database`; + ? `${userDescPrefix}Search and list database objects (schemas, tables, columns, procedures, functions, indexes) on the ${dbType} database` + : `${userDescPrefix}Search and list database objects (schemas, tables, columns, procedures, functions, indexes) on the '${sourceId}' ${dbType} database`; return { name: toolName, From 055f2a505bb5c65cf1111c45b6a44bdf1efddd60 Mon Sep 17 00:00:00 2001 From: IslaLiu Date: Tue, 21 Apr 2026 17:45:45 +0800 Subject: [PATCH 2/3] refactor: extract buildSourceDescriptionPrefix helper and normalize input Addresses Copilot review feedback on PR #309: - Extract prefix-building logic into a pure helper `buildSourceDescriptionPrefix` used by both `getExecuteSqlMetadata` and `getSearchObjectsMetadata` instead of inlining the same expression twice. - Trim surrounding whitespace from the user-provided description so leading or trailing whitespace does not leak into the MCP tool description. - Skip appending '.' when the description already ends with a sentence-ending character (one of '.', '!', '?') to avoid double punctuation such as "Prod DB.. Execute SQL...". Adds 10 pure unit tests in src/utils/__tests__/tool-metadata.test.ts covering undefined/empty/whitespace-only inputs, each sentence-ending punctuation case, whitespace trimming, preservation of internal formatting, and documenting that mid-sentence punctuation (':', ';', ')') is still terminated with '. '. --- src/utils/__tests__/tool-metadata.test.ts | 54 +++++++++++++++++++++++ src/utils/tool-metadata.ts | 25 ++++++++++- 2 files changed, 77 insertions(+), 2 deletions(-) create mode 100644 src/utils/__tests__/tool-metadata.test.ts diff --git a/src/utils/__tests__/tool-metadata.test.ts b/src/utils/__tests__/tool-metadata.test.ts new file mode 100644 index 00000000..bbe0e329 --- /dev/null +++ b/src/utils/__tests__/tool-metadata.test.ts @@ -0,0 +1,54 @@ +import { describe, it, expect } from "vitest"; +import { buildSourceDescriptionPrefix } from "../tool-metadata.js"; + +describe("buildSourceDescriptionPrefix", () => { + it("returns empty string when description is undefined", () => { + expect(buildSourceDescriptionPrefix(undefined)).toBe(""); + }); + + it("returns empty string when description is empty", () => { + expect(buildSourceDescriptionPrefix("")).toBe(""); + }); + + it("returns empty string when description is whitespace-only", () => { + expect(buildSourceDescriptionPrefix(" ")).toBe(""); + expect(buildSourceDescriptionPrefix("\t\n")).toBe(""); + }); + + it("appends '. ' when description has no sentence-ending punctuation", () => { + expect(buildSourceDescriptionPrefix("Prod DB")).toBe("Prod DB. "); + }); + + it("appends only a space when description already ends with a period", () => { + // Guards against the classic "Prod DB.. Execute SQL..." double-period bug. + expect(buildSourceDescriptionPrefix("Prod DB.")).toBe("Prod DB. "); + }); + + it("appends only a space when description ends with '!'", () => { + expect(buildSourceDescriptionPrefix("Production DB!")).toBe("Production DB! "); + }); + + it("appends only a space when description ends with '?'", () => { + expect(buildSourceDescriptionPrefix("Query me?")).toBe("Query me? "); + }); + + it("trims surrounding whitespace before assessing punctuation", () => { + expect(buildSourceDescriptionPrefix(" Prod DB ")).toBe("Prod DB. "); + expect(buildSourceDescriptionPrefix(" Prod DB. ")).toBe("Prod DB. "); + }); + + it("preserves internal whitespace and punctuation", () => { + // Internal formatting (newlines, multiple words, mid-sentence punctuation) + // is the user's intent and must not be altered. + expect(buildSourceDescriptionPrefix("Line 1\nLine 2")).toBe("Line 1\nLine 2. "); + expect(buildSourceDescriptionPrefix("Line A, Line B")).toBe("Line A, Line B. "); + }); + + it("does not treat non-sentence-ending punctuation as terminators", () => { + // These are mid-sentence / structural punctuation; the helper should + // still append ". " to produce a complete sentence boundary. + expect(buildSourceDescriptionPrefix("Details below:")).toBe("Details below:. "); + expect(buildSourceDescriptionPrefix("(read-only)")).toBe("(read-only). "); + expect(buildSourceDescriptionPrefix("Clause 1; clause 2")).toBe("Clause 1; clause 2. "); + }); +}); diff --git a/src/utils/tool-metadata.ts b/src/utils/tool-metadata.ts index bfdf8528..fe24faad 100644 --- a/src/utils/tool-metadata.ts +++ b/src/utils/tool-metadata.ts @@ -39,6 +39,27 @@ export interface ToolMetadata { annotations: ToolAnnotations; } +/** + * Build a prefix string for prepending a source's user-provided `description` + * onto a generated tool description. Returns "" when no description is set + * (undefined, empty, or whitespace-only). Normalizes surrounding whitespace + * with `trim()` and avoids double sentence-ending punctuation when the + * description already ends with one of "." / "!" / "?". + * + * Examples: + * undefined -> "" + * " " -> "" + * "Prod DB" -> "Prod DB. " + * "Prod DB." -> "Prod DB. " (no double period) + * " Prod DB! " -> "Prod DB! " (trimmed, no added period) + * "Query me?" -> "Query me? " + */ +export function buildSourceDescriptionPrefix(description: string | undefined): string { + const trimmed = description?.trim() ?? ""; + if (!trimmed) return ""; + return /[.!?]$/.test(trimmed) ? `${trimmed} ` : `${trimmed}. `; +} + /** * Convert a Zod schema object to simplified parameter list * @param schema - Zod schema object (e.g., { sql: z.string().describe("...") }) @@ -109,7 +130,7 @@ export function getExecuteSqlMetadata(sourceId: string): ToolMetadata { // Determine description with more context. // Prepend the user-provided `description` from the source config (if set) // so AI clients reading the MCP tool list see the source's purpose first. - const userDescPrefix = sourceConfig.description ? `${sourceConfig.description}. ` : ""; + const userDescPrefix = buildSourceDescriptionPrefix(sourceConfig.description); const readonlyNote = executeOptions.readonly ? " [READ-ONLY MODE]" : ""; const maxRowsNote = executeOptions.maxRows ? ` (limited to ${executeOptions.maxRows} rows)` : ""; const description = isSingleSource @@ -154,7 +175,7 @@ export function getSearchObjectsMetadata(sourceId: string): { name: string; desc : `Search Database Objects on ${sourceId} (${dbType})`; // Prepend the user-provided `description` from the source config (if set) // so AI clients reading the MCP tool list see the source's purpose first. - const userDescPrefix = sourceConfig.description ? `${sourceConfig.description}. ` : ""; + const userDescPrefix = buildSourceDescriptionPrefix(sourceConfig.description); const description = isSingleSource ? `${userDescPrefix}Search and list database objects (schemas, tables, columns, procedures, functions, indexes) on the ${dbType} database` : `${userDescPrefix}Search and list database objects (schemas, tables, columns, procedures, functions, indexes) on the '${sourceId}' ${dbType} database`; From 8d631918ba802336a62a83f1c349f27fca8f5a5b Mon Sep 17 00:00:00 2001 From: IslaLiu Date: Tue, 21 Apr 2026 17:57:26 +0800 Subject: [PATCH 3/3] refactor: treat trailing ':' as terminator to avoid ':.' artifact Addresses second-pass Copilot review on PR #309: descriptions ending with a colon (e.g. "Details below:") produced the awkward artifact "Details below:. Execute SQL..." under the previous regex. A trailing colon naturally introduces what follows (the generic tool template), so appending "." after it is both ugly and semantically redundant. Extend the terminator regex to /[.!?:]$/ and update the corresponding test case. ';' and ')' remain non-terminators because they signal mid-sentence continuation rather than introduction. --- src/utils/__tests__/tool-metadata.test.ts | 12 +++++++++--- src/utils/tool-metadata.ts | 12 +++++++++--- 2 files changed, 18 insertions(+), 6 deletions(-) diff --git a/src/utils/__tests__/tool-metadata.test.ts b/src/utils/__tests__/tool-metadata.test.ts index bbe0e329..19e82c1c 100644 --- a/src/utils/__tests__/tool-metadata.test.ts +++ b/src/utils/__tests__/tool-metadata.test.ts @@ -32,6 +32,12 @@ describe("buildSourceDescriptionPrefix", () => { expect(buildSourceDescriptionPrefix("Query me?")).toBe("Query me? "); }); + it("appends only a space when description ends with ':'", () => { + // A trailing colon naturally introduces what follows (the tool template), + // so adding '.' here would produce the artifact "Details below:. Execute..." + expect(buildSourceDescriptionPrefix("Details below:")).toBe("Details below: "); + }); + it("trims surrounding whitespace before assessing punctuation", () => { expect(buildSourceDescriptionPrefix(" Prod DB ")).toBe("Prod DB. "); expect(buildSourceDescriptionPrefix(" Prod DB. ")).toBe("Prod DB. "); @@ -45,9 +51,9 @@ describe("buildSourceDescriptionPrefix", () => { }); it("does not treat non-sentence-ending punctuation as terminators", () => { - // These are mid-sentence / structural punctuation; the helper should - // still append ". " to produce a complete sentence boundary. - expect(buildSourceDescriptionPrefix("Details below:")).toBe("Details below:. "); + // ')' and ';' are mid-sentence / structural punctuation; the helper + // should still append ". " to produce a complete sentence boundary. + // (':' is handled separately — see the colon-specific test above.) expect(buildSourceDescriptionPrefix("(read-only)")).toBe("(read-only). "); expect(buildSourceDescriptionPrefix("Clause 1; clause 2")).toBe("Clause 1; clause 2. "); }); diff --git a/src/utils/tool-metadata.ts b/src/utils/tool-metadata.ts index fe24faad..64225209 100644 --- a/src/utils/tool-metadata.ts +++ b/src/utils/tool-metadata.ts @@ -43,8 +43,11 @@ export interface ToolMetadata { * Build a prefix string for prepending a source's user-provided `description` * onto a generated tool description. Returns "" when no description is set * (undefined, empty, or whitespace-only). Normalizes surrounding whitespace - * with `trim()` and avoids double sentence-ending punctuation when the - * description already ends with one of "." / "!" / "?". + * with `trim()` and skips adding a period when the description already ends + * with one of "." / "!" / "?" / ":" — the colon is included because a + * trailing colon naturally introduces what follows (the generic tool + * template) and appending "." after it would produce artifacts like + * "Details below:. Execute SQL...". * * Examples: * undefined -> "" @@ -53,11 +56,14 @@ export interface ToolMetadata { * "Prod DB." -> "Prod DB. " (no double period) * " Prod DB! " -> "Prod DB! " (trimmed, no added period) * "Query me?" -> "Query me? " + * "Details below:" -> "Details below: " (colon introduces what follows) + * "Clause 1; clause 2" -> "Clause 1; clause 2. " (semicolon is mid-sentence) + * "(read-only)" -> "(read-only). " (closing paren is mid-sentence) */ export function buildSourceDescriptionPrefix(description: string | undefined): string { const trimmed = description?.trim() ?? ""; if (!trimmed) return ""; - return /[.!?]$/.test(trimmed) ? `${trimmed} ` : `${trimmed}. `; + return /[.!?:]$/.test(trimmed) ? `${trimmed} ` : `${trimmed}. `; } /**