From 3883ae93217118506fb3d30723a77e764443fb08 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 19 Sep 2026 16:26:41 +0000 Subject: [PATCH 1/2] feat: implement health_check for Oracle Connection pool state from V$SESSION (user sessions other than the diagnostic one; TADDR marks an open transaction, LAST_CALL_ET gives the current call's or idle period's age) with the ceiling from the `sessions` parameter; buffer cache hit ratio from V$SYSSTAT (db block gets + consistent gets vs physical reads). Each section degrades to a `notes` entry when the connecting user lacks SELECT_CATALOG_ROLE / SELECT ANY DICTIONARY, matching the SQL Server connector's posture. Integration test accepts either populated metrics or the explanatory note; docs, CLAUDE.md and the TOML example list Oracle as supported. Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_0189BHv585xi8iqEp9JvmgKY --- CLAUDE.md | 2 +- dbhub.toml.example | 11 ++- docs/tools/health-check.mdx | 7 +- docs/tools/overview.mdx | 2 +- .../__tests__/oracle.integration.test.ts | 30 ++++++ src/connectors/oracle/index.ts | 96 +++++++++++++++++++ 6 files changed, 138 insertions(+), 10 deletions(-) diff --git a/CLAUDE.md b/CLAUDE.md index 4ee794b2..ae930afa 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -35,7 +35,7 @@ src/ │ ├── execute-sql.ts # SQL execution handler │ ├── search-objects.ts # Unified search/list with progressive disclosure │ ├── explain-sql.ts # Opt-in EXPLAIN plan tool (never executes the target statement) -│ └── health-check.ts # Opt-in connection pool + buffer cache metrics tool (Postgres/MySQL/MariaDB/SQL Server) +│ └── health-check.ts # Opt-in connection pool + buffer cache metrics tool (Postgres/MySQL/MariaDB/SQL Server/Oracle) ├── utils/ # Shared utilities │ ├── dsn-obfuscator.ts# DSN security │ ├── response-formatter.ts # Output formatting diff --git a/dbhub.toml.example b/dbhub.toml.example index b1f543d0..12f34fde 100644 --- a/dbhub.toml.example +++ b/dbhub.toml.example @@ -272,11 +272,12 @@ dsn = "postgres://postgres:postgres@localhost:5432/myapp" # 'health_check' is opt-in only, like explain_sql. It reports operational # metrics (connection pool state, buffer cache hit ratio) for the source. -# Currently implemented for PostgreSQL, MySQL, MariaDB, and SQL Server; -# enabling it on other database types returns an "unsupported" error rather -# than partial data. On MySQL/MariaDB, idle-in-transaction detection +# Currently implemented for PostgreSQL, MySQL, MariaDB, SQL Server, and +# Oracle; enabling it on other database types returns an "unsupported" error +# rather than partial data. On MySQL/MariaDB, idle-in-transaction detection # additionally requires the PROCESS privilege; on SQL Server, both metrics -# require VIEW SERVER STATE (VIEW DATABASE STATE on Azure SQL Database). +# require VIEW SERVER STATE (VIEW DATABASE STATE on Azure SQL Database); on +# Oracle, both require SELECT_CATALOG_ROLE (or SELECT ANY DICTIONARY). # Without the right grant, the tool still returns what it can and adds a # `notes` entry explaining the reduced visibility instead of failing # outright. Always read-only, no readonly/max_rows options of its own. @@ -420,7 +421,7 @@ dsn = "postgres://postgres:postgres@localhost:5432/myapp" # readonly = true # Restrict to SELECT, SHOW, DESCRIBE, EXPLAIN (works for execute_sql and custom tools) # max_rows = 1000 # Limit result set size (works for execute_sql and custom tools) # explain_sql # Opt-in tool (name = "explain_sql"); no readonly/max_rows options - always safe -# health_check # Opt-in tool (name = "health_check"); PostgreSQL/MySQL/MariaDB/SQL Server for now, always safe +# health_check # Opt-in tool (name = "health_check"); PostgreSQL/MySQL/MariaDB/SQL Server/Oracle for now, always safe # # Parameter Placeholders by Database: # PostgreSQL: $1, $2, $3 diff --git a/docs/tools/health-check.mdx b/docs/tools/health-check.mdx index e37de8b2..59d77c08 100644 --- a/docs/tools/health-check.mdx +++ b/docs/tools/health-check.mdx @@ -8,8 +8,8 @@ Report operational health metrics for a database source: connection pool state a - **Connection pool state**: Total/active/idle session counts, idle-in-transaction sessions, the configured connection ceiling, and how long the longest-running query or idle-in-transaction session has been open - **Buffer cache hit ratio**: Percentage of reads served from cache vs disk, useful for spotting an undersized cache before it becomes a production incident -- **Per-engine support**: Implemented for PostgreSQL, MySQL, MariaDB, and SQL Server. SQLite has no connection pool or cache-hit concept to report, and Oracle is not implemented yet, so `health_check` returns an `UNSUPPORTED` error there -- **Graceful degradation**: On MySQL/MariaDB/SQL Server, some metrics require an elevated privilege the connected user may not have. Rather than failing outright, `health_check` returns whatever it can and adds a `notes` entry explaining what's missing +- **Per-engine support**: Implemented for PostgreSQL, MySQL, MariaDB, SQL Server, and Oracle. SQLite has no connection pool or cache-hit concept to report, so `health_check` returns an `UNSUPPORTED` error there +- **Graceful degradation**: On MySQL/MariaDB/SQL Server/Oracle, some metrics require an elevated privilege the connected user may not have. Rather than failing outright, `health_check` returns whatever it can and adds a `notes` entry explaining what's missing - **Opt-in only**: Not part of the default tool pair — must be explicitly enabled per source @@ -48,7 +48,7 @@ Call the tool with no arguments — metrics are always for the source the tool i ### Reduced-privilege output -On MySQL, MariaDB, and SQL Server, some metrics need a privilege the connected user might not have — the tool still returns what it can and explains the gap instead of erroring: +On MySQL, MariaDB, SQL Server, and Oracle, some metrics need a privilege the connected user might not have — the tool still returns what it can and explains the gap instead of erroring: ```json Example output (MySQL, without PROCESS privilege) { @@ -68,6 +68,7 @@ On MySQL, MariaDB, and SQL Server, some metrics need a privilege the connected u | --- | --- | --- | | MySQL / MariaDB | `PROCESS` | Idle-in-transaction detection only; connection/buffer-cache counts are still returned, but without it, connection visibility is restricted to the caller's own sessions (and the diagnostic session itself is excluded), so counts may under-report down to 0 | | SQL Server | `VIEW SERVER STATE` (`VIEW DATABASE STATE` on Azure SQL Database) | Both connection pool and buffer cache sections | +| Oracle | `SELECT_CATALOG_ROLE` (or `SELECT ANY DICTIONARY`) | Both sections: connection pool from `V$SESSION` / `V$PARAMETER`, buffer cache from `V$SYSSTAT` | | PostgreSQL | None | All metrics are available to any connected user | ## Enabling health_check diff --git a/docs/tools/overview.mdx b/docs/tools/overview.mdx index 0d22ee9b..beef8958 100644 --- a/docs/tools/overview.mdx +++ b/docs/tools/overview.mdx @@ -9,7 +9,7 @@ title: "Overview" | Execute SQL | `execute_sql` or `execute_sql_{id}` | On | Execute single or multiple SQL statements (separated by semicolons) | | Search Objects | `search_objects` or `search_objects_{id}` | On | Search and list database objects (schemas, tables, columns, procedures, indexes) with pattern matching and token-efficient progressive disclosure | | Explain SQL | `explain_sql` or `explain_sql_{id}` | Opt-in | Show the execution plan for a SQL statement without running it | -| Health Check | `health_check` or `health_check_{id}` | Opt-in | Report connection pool state and buffer cache hit ratio (PostgreSQL, MySQL, MariaDB, SQL Server) | +| Health Check | `health_check` or `health_check_{id}` | Opt-in | Report connection pool state and buffer cache hit ratio (PostgreSQL, MySQL, MariaDB, SQL Server, Oracle) | | Custom Tools | User-defined names | Opt-in | Define reusable, parameterized SQL operations in your `dbhub.toml` configuration file | ## Tool Configuration diff --git a/src/connectors/__tests__/oracle.integration.test.ts b/src/connectors/__tests__/oracle.integration.test.ts index dff470fc..9fc38e02 100644 --- a/src/connectors/__tests__/oracle.integration.test.ts +++ b/src/connectors/__tests__/oracle.integration.test.ts @@ -338,6 +338,36 @@ describe('Oracle Connector Integration Tests', () => { }); }); + describe('Oracle-specific: health check', () => { + it('reports connection pool state and buffer cache hit ratio, or explains what it cannot read', async () => { + const health = await oracleTest.connector.getHealthCheck!(); + + // The container's application user may or may not hold + // SELECT_CATALOG_ROLE; either way every section is populated or + // accounted for by a note, never silently missing. + if (health.connections) { + expect(health.connections.total).toBeGreaterThanOrEqual(0); + expect(health.connections.active).toBeGreaterThanOrEqual(0); + expect(health.connections.idle).toBeGreaterThanOrEqual(0); + expect(health.connections.idleInTransaction).toBeGreaterThanOrEqual(0); + expect(health.connections.idleInTransactionAborted).toBeUndefined(); + expect(health.connections.maxConnections).toBeGreaterThan(0); + } else { + expect(health.notes).toEqual(expect.arrayContaining([expect.stringContaining('V$SESSION')])); + } + + if (health.bufferCache) { + expect(health.bufferCache.blocksHit + health.bufferCache.blocksRead).toBeGreaterThan(0); + if (health.bufferCache.hitRatioPct !== null) { + expect(health.bufferCache.hitRatioPct).toBeGreaterThanOrEqual(0); + expect(health.bufferCache.hitRatioPct).toBeLessThanOrEqual(100); + } + } else { + expect(health.notes).toEqual(expect.arrayContaining([expect.stringContaining('V$SYSSTAT')])); + } + }); + }); + describe('Oracle-specific: EXPLAIN', () => { it('returns an execution plan for a bare EXPLAIN without executing the statement', async () => { const result = await oracleTest.connector.executeSQL('EXPLAIN SELECT * FROM users WHERE id = 1', { readonly: true }); diff --git a/src/connectors/oracle/index.ts b/src/connectors/oracle/index.ts index 6e9e9609..55b9d73a 100644 --- a/src/connectors/oracle/index.ts +++ b/src/connectors/oracle/index.ts @@ -11,7 +11,9 @@ import { StoredProcedure, ExecuteOptions, ConnectorConfig, + HealthCheckResult, } from "../interface.js"; +import { computeHitRatioPct, toNullableNumber } from "../health-check-utils.js"; import { SafeURL } from "../../utils/safe-url.js"; import { obfuscateDSNPassword } from "../../utils/dsn-obfuscate.js"; import { SQLRowLimiter } from "../../utils/sql-row-limiter.js"; @@ -516,6 +518,100 @@ export class OracleConnector implements Connector { } } + async getHealthCheck(): Promise { + if (!this.pool) { + throw new Error("Not connected to Oracle database"); + } + + const notes: string[] = []; + const result: HealthCheckResult = {}; + + // V$SESSION / V$PARAMETER / V$SYSSTAT are readable only with + // SELECT_CATALOG_ROLE (or SELECT ANY DICTIONARY); without it Oracle + // reports ORA-00942 as if the view did not exist. Degrade per section + // instead of failing the whole health check. + try { + const [sessions, params] = await this.withConnection((connection) => + Promise.all([ + OracleConnector.fetchRows<{ + TOTAL: number; + ACTIVE: number; + IDLE: number; + IDLE_IN_TRANSACTION: number; + LONGEST_IDLE_IN_TRANSACTION_SECONDS: number | null; + LONGEST_ACTIVE_QUERY_SECONDS: number | null; + }>( + connection, + // TADDR is non-null while the session has an open transaction; + // LAST_CALL_ET is seconds since the current call began (ACTIVE) + // or since the last call ended (otherwise). + `SELECT + COUNT(*) AS total, + SUM(CASE WHEN status = 'ACTIVE' THEN 1 ELSE 0 END) AS active, + SUM(CASE WHEN status <> 'ACTIVE' THEN 1 ELSE 0 END) AS idle, + SUM(CASE WHEN status <> 'ACTIVE' AND taddr IS NOT NULL THEN 1 ELSE 0 END) AS idle_in_transaction, + MAX(CASE WHEN status <> 'ACTIVE' AND taddr IS NOT NULL THEN last_call_et END) AS longest_idle_in_transaction_seconds, + MAX(CASE WHEN status = 'ACTIVE' THEN last_call_et END) AS longest_active_query_seconds + FROM v$session + WHERE type = 'USER' + AND sid <> SYS_CONTEXT('USERENV', 'SID')` + ), + OracleConnector.fetchRows<{ VALUE: string }>( + connection, + `SELECT value FROM v$parameter WHERE name = 'sessions'` + ), + ]) + ); + const conn = sessions[0]; + const maxConnections = params.length > 0 ? Number(params[0].VALUE) : null; + + result.connections = { + total: Number(conn.TOTAL ?? 0), + active: Number(conn.ACTIVE ?? 0), + idle: Number(conn.IDLE ?? 0), + idleInTransaction: Number(conn.IDLE_IN_TRANSACTION ?? 0), + // Oracle has no equivalent of Postgres's "idle in transaction + // (aborted)" state: a failed statement is rolled back on its own + // and leaves the transaction usable. + maxConnections: maxConnections !== null && maxConnections > 0 ? maxConnections : null, + longestIdleInTransactionSeconds: toNullableNumber(conn.LONGEST_IDLE_IN_TRANSACTION_SECONDS), + longestActiveQuerySeconds: toNullableNumber(conn.LONGEST_ACTIVE_QUERY_SECONDS), + }; + } catch { + notes.push( + "Connection pool metrics unavailable: connecting user lacks SELECT on V$SESSION / V$PARAMETER (grant SELECT_CATALOG_ROLE or SELECT ANY DICTIONARY)." + ); + } + + try { + const stats = await this.query<{ NAME: string; VALUE: number }>( + `SELECT name, value FROM v$sysstat + WHERE name IN ('db block gets', 'consistent gets', 'physical reads')` + ); + const byName = Object.fromEntries(stats.map((row) => [row.NAME, Number(row.VALUE)])); + // Logical reads = current-mode gets + consistent-mode gets; physical + // reads are the subset that had to go to disk. + const logicalReads = (byName["db block gets"] ?? 0) + (byName["consistent gets"] ?? 0); + const physicalReads = byName["physical reads"] ?? 0; + + result.bufferCache = { + hitRatioPct: computeHitRatioPct(logicalReads, physicalReads), + blocksHit: logicalReads - physicalReads, + blocksRead: physicalReads, + }; + } catch { + notes.push( + "Buffer cache metrics unavailable: connecting user lacks SELECT on V$SYSSTAT (grant SELECT_CATALOG_ROLE or SELECT ANY DICTIONARY)." + ); + } + + if (notes.length > 0) { + result.notes = notes; + } + + return result; + } + async getStoredProcedures(schema?: string, routineType?: "procedure" | "function"): Promise { try { const typeFilter = From 0f8a63194aa58ead378c7a2f2fc2905aa9913fe1 Mon Sep 17 00:00:00 2001 From: Claude Date: Sat, 19 Sep 2026 16:36:09 +0000 Subject: [PATCH 2/2] fix: address Copilot review on Oracle health_check - Count only INACTIVE sessions as idle; KILLED / SNIPED / CACHED are transitional states that belong in the total but are neither active nor idle - Deterministic integration tests: connect as SYSTEM (same password as the app user in the test image) to grant SELECT_CATALOG_ROLE to the app user and create a user without it, then assert the populated path and the notes-only path separately - skills/dbhub/SKILL.md lists Oracle for health_check Co-Authored-By: Claude Fable 5.1 Claude-Session: https://claude.ai/code/session_0189BHv585xi8iqEp9JvmgKY --- skills/dbhub/SKILL.md | 2 +- .../__tests__/oracle.integration.test.ts | 79 +++++++++++++------ src/connectors/oracle/index.ts | 11 ++- 3 files changed, 64 insertions(+), 28 deletions(-) diff --git a/skills/dbhub/SKILL.md b/skills/dbhub/SKILL.md index 7bf04879..03e775cc 100644 --- a/skills/dbhub/SKILL.md +++ b/skills/dbhub/SKILL.md @@ -16,7 +16,7 @@ DBHub provides two MCP tools by default, plus opt-in ones: | `search_objects` | Explore database structure — schemas, tables, columns, indexes, procedures, functions | | `execute_sql` | Run SQL statements against the database | | `explain_sql` (opt-in) | Show a query's execution plan without running it — only present if the source's config enables it | -| `health_check` (opt-in) | Report connection pool state and buffer cache hit ratio — only present if the source's config enables it; PostgreSQL, MySQL, MariaDB, and SQL Server only | +| `health_check` (opt-in) | Report connection pool state and buffer cache hit ratio — only present if the source's config enables it; PostgreSQL, MySQL, MariaDB, SQL Server, and Oracle only | If multiple databases are configured, DBHub registers separate tools for each source (for example, `search_objects_prod_pg`, `execute_sql_staging_mysql`). Select the desired database by calling the correspondingly named tool. diff --git a/src/connectors/__tests__/oracle.integration.test.ts b/src/connectors/__tests__/oracle.integration.test.ts index 9fc38e02..80ed5636 100644 --- a/src/connectors/__tests__/oracle.integration.test.ts +++ b/src/connectors/__tests__/oracle.integration.test.ts @@ -339,32 +339,65 @@ describe('Oracle Connector Integration Tests', () => { }); describe('Oracle-specific: health check', () => { - it('reports connection pool state and buffer cache hit ratio, or explains what it cannot read', async () => { + // The image gives SYSTEM the same password as the application user, so + // the test can shape privileges deterministically: grant the app user + // the catalog role (privileged path) and create a user without it + // (restricted path). + const RESTRICTED_USER = 'dbhub_restricted'; + let restricted: Connector; + + beforeAll(async () => { + const admin = new OracleConnector(); + await admin.connect(oracleTest.connectionString.replace(`${APP_USER}:${APP_PASSWORD}@`, `system:${APP_PASSWORD}@`)); + try { + await admin.executeSQL(`GRANT SELECT_CATALOG_ROLE TO ${APP_USER}`, {}); + await admin.executeSQL(`CREATE USER ${RESTRICTED_USER} IDENTIFIED BY "${APP_PASSWORD}"`, {}); + await admin.executeSQL(`GRANT CREATE SESSION TO ${RESTRICTED_USER}`, {}); + } finally { + await admin.disconnect(); + } + + // A fresh pool so the new role applies to every session it opens. + await oracleTest.connector.disconnect(); + await oracleTest.connector.connect(oracleTest.connectionString); + + restricted = new OracleConnector(); + await restricted.connect(oracleTest.connectionString.replace(`${APP_USER}:${APP_PASSWORD}@`, `${RESTRICTED_USER}:${APP_PASSWORD}@`)); + }); + + afterAll(async () => { + await restricted?.disconnect(); + }); + + it('reports connection pool state and buffer cache hit ratio with SELECT_CATALOG_ROLE', async () => { const health = await oracleTest.connector.getHealthCheck!(); + expect(health.notes).toBeUndefined(); + + expect(health.connections).toBeDefined(); + expect(health.connections!.total).toBeGreaterThanOrEqual(0); + expect(health.connections!.active).toBeGreaterThanOrEqual(0); + expect(health.connections!.idle).toBeGreaterThanOrEqual(0); + expect(health.connections!.active + health.connections!.idle).toBeLessThanOrEqual(health.connections!.total); + expect(health.connections!.idleInTransaction).toBeGreaterThanOrEqual(0); + expect(health.connections!.idleInTransactionAborted).toBeUndefined(); + expect(health.connections!.maxConnections).toBeGreaterThan(0); + + expect(health.bufferCache).toBeDefined(); + expect(health.bufferCache!.blocksHit + health.bufferCache!.blocksRead).toBeGreaterThan(0); + expect(health.bufferCache!.hitRatioPct).not.toBeNull(); + expect(health.bufferCache!.hitRatioPct).toBeGreaterThanOrEqual(0); + expect(health.bufferCache!.hitRatioPct).toBeLessThanOrEqual(100); + }); - // The container's application user may or may not hold - // SELECT_CATALOG_ROLE; either way every section is populated or - // accounted for by a note, never silently missing. - if (health.connections) { - expect(health.connections.total).toBeGreaterThanOrEqual(0); - expect(health.connections.active).toBeGreaterThanOrEqual(0); - expect(health.connections.idle).toBeGreaterThanOrEqual(0); - expect(health.connections.idleInTransaction).toBeGreaterThanOrEqual(0); - expect(health.connections.idleInTransactionAborted).toBeUndefined(); - expect(health.connections.maxConnections).toBeGreaterThan(0); - } else { - expect(health.notes).toEqual(expect.arrayContaining([expect.stringContaining('V$SESSION')])); - } + it('degrades to notes, not an error, without the catalog role', async () => { + const health = await restricted.getHealthCheck!(); - if (health.bufferCache) { - expect(health.bufferCache.blocksHit + health.bufferCache.blocksRead).toBeGreaterThan(0); - if (health.bufferCache.hitRatioPct !== null) { - expect(health.bufferCache.hitRatioPct).toBeGreaterThanOrEqual(0); - expect(health.bufferCache.hitRatioPct).toBeLessThanOrEqual(100); - } - } else { - expect(health.notes).toEqual(expect.arrayContaining([expect.stringContaining('V$SYSSTAT')])); - } + expect(health.connections).toBeUndefined(); + expect(health.bufferCache).toBeUndefined(); + expect(health.notes).toEqual([ + expect.stringContaining('V$SESSION'), + expect.stringContaining('V$SYSSTAT'), + ]); }); }); diff --git a/src/connectors/oracle/index.ts b/src/connectors/oracle/index.ts index 55b9d73a..50ba488d 100644 --- a/src/connectors/oracle/index.ts +++ b/src/connectors/oracle/index.ts @@ -542,15 +542,18 @@ export class OracleConnector implements Connector { LONGEST_ACTIVE_QUERY_SECONDS: number | null; }>( connection, - // TADDR is non-null while the session has an open transaction; + // STATUS is ACTIVE (running a call), INACTIVE (idle), or one of + // the transitional states KILLED / SNIPED / CACHED, which count + // toward the total but are neither active nor idle. TADDR is + // non-null while the session has an open transaction; // LAST_CALL_ET is seconds since the current call began (ACTIVE) // or since the last call ended (otherwise). `SELECT COUNT(*) AS total, SUM(CASE WHEN status = 'ACTIVE' THEN 1 ELSE 0 END) AS active, - SUM(CASE WHEN status <> 'ACTIVE' THEN 1 ELSE 0 END) AS idle, - SUM(CASE WHEN status <> 'ACTIVE' AND taddr IS NOT NULL THEN 1 ELSE 0 END) AS idle_in_transaction, - MAX(CASE WHEN status <> 'ACTIVE' AND taddr IS NOT NULL THEN last_call_et END) AS longest_idle_in_transaction_seconds, + SUM(CASE WHEN status = 'INACTIVE' THEN 1 ELSE 0 END) AS idle, + SUM(CASE WHEN status = 'INACTIVE' AND taddr IS NOT NULL THEN 1 ELSE 0 END) AS idle_in_transaction, + MAX(CASE WHEN status = 'INACTIVE' AND taddr IS NOT NULL THEN last_call_et END) AS longest_idle_in_transaction_seconds, MAX(CASE WHEN status = 'ACTIVE' THEN last_call_et END) AS longest_active_query_seconds FROM v$session WHERE type = 'USER'