Skip to content

feat: implement health_check for Oracle - #435

Merged
tianzhou merged 2 commits into
mainfrom
claude/bold-hawking-2n28vd
Sep 19, 2026
Merged

tianzhou merged 2 commits into
mainfrom
claude/bold-hawking-2n28vd

Conversation

@tianzhou

@tianzhou tianzhou commented Sep 19, 2026 •

Copy link
Copy Markdown
Member

Closes the one gap #433 called out: health_check now works on Oracle sources.

Implementation (src/connectors/oracle/index.ts)

  • Connection pool from V$SESSION, counting user sessions other than the diagnostic one. ACTIVE sessions are active, INACTIVE ones idle; the transitional KILLED / SNIPED / CACHED states count toward the total only. TADDR non-null marks an open transaction (idle-in-transaction); LAST_CALL_ET gives the age of the current call for active sessions or of the idle period otherwise. The ceiling is the sessions parameter from V$PARAMETER. idle_in_transaction_aborted is omitted: Oracle rolls back a failed statement on its own and leaves the transaction usable, so there is no aborted-but-open state.
  • Buffer cache from V$SYSSTAT: logical reads are db block gets + consistent gets, physical reads are physical reads, ratio via the shared computeHitRatioPct.
  • Graceful degradation, matching the SQL Server connector: without SELECT_CATALOG_ROLE / SELECT ANY DICTIONARY Oracle reports the V$ views as non-existent, so each section falls back to a notes entry naming the grant instead of failing the whole call.

Tests and docs

  • Deterministic integration tests. The test image gives SYSTEM the same password as the application user, so the suite grants SELECT_CATALOG_ROLE to the app user and creates a second user without it. The privileged path asserts both sections populated with no notes; the restricted path asserts no sections and exactly the two notes. Both pass in CI.
  • docs/tools/health-check.mdx (features, privilege table row), docs/tools/overview.mdx, skills/dbhub/SKILL.md, CLAUDE.md, and dbhub.toml.example list Oracle as supported.

Review

Copilot's one inline finding and two summary-only findings (session status handling, deterministic tests) are addressed in the second commit.

🤖 Generated with Claude Code

https://claude.ai/code/session_0189BHv585xi8iqEp9JvmgKY

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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0189BHv585xi8iqEp9JvmgKY
Copilot AI lite review requested due to automatic review settings September 19, 2026 16:28

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copilot review overview

🟡 Changes recommended

Moderate implementation and test-coverage issues remain, and the skill support matrix is inconsistent.

Get a fresh assessment by requesting another Copilot review.

Review effort: Lite
Findings: 1 Low severity

Open (1)
What changed in this PR

Adds Oracle support to the opt-in health_check tool using dynamic performance views with privilege-aware fallback behavior.

Changes:

  • Implements Oracle session and buffer-cache metrics.
  • Adds Oracle integration coverage.
  • Updates documentation and configuration examples.
File Summary
src/​connectors/​oracle/​index.ts Implements Oracle health metrics and fallback notes. Moderate finding (1 vote): handle V$SESSION.STATUS values beyond ACTIVE and INACTIVE.
src/​connectors/​__tests__/​oracle.integration.test.ts Adds health-check integration coverage. Moderate findings (1 vote each): add deterministic privileged-path and restricted-privilege tests.
docs/​tools/​overview.mdx Lists Oracle health-check support.
docs/​tools/​health-check.mdx Documents Oracle metrics and privileges. Nit (2 votes): also update skills/dbhub/SKILL.md to avoid contradictory support guidance.
dbhub.toml.example Updates health-check configuration guidance.
CLAUDE.md Updates architecture support documentation.

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.

Comment thread docs/tools/health-check.mdx
- 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 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_0189BHv585xi8iqEp9JvmgKY
@tianzhou
tianzhou merged commit f6acb89 into main Sep 19, 2026
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants