Skip to content

refactor: structured SQLResult.messages (DatabaseMessage[])#327

Merged
tianzhou merged 1 commit into
mainfrom
feat/structured-db-messages
Jun 7, 2026
Merged

refactor: structured SQLResult.messages (DatabaseMessage[])#327
tianzhou merged 1 commit into
mainfrom
feat/structured-db-messages

Conversation

@tianzhou

@tianzhou tianzhou commented Jun 7, 2026

Copy link
Copy Markdown
Member

Summary

Follow-up to #307. That PR added SQLResult.messages?: string[] to surface SQL Server informational output (SET STATISTICS TIME/IO, PRINT, warnings) to MCP clients. This PR promotes that field from a bare string[] to a structured DatabaseMessage[] so per-message metadata is preserved instead of flattened away.

Why

string[] is lossy. The mssql info event carries message, class (severity), number, and lineNumber — all discarded by the string form. Most importantly, severity lets a client tell a benign PRINT from an actual warning.

It's also a genuinely cross-database concept, not a SQL-Server quirk: PostgreSQL emits NOTICE/WARNING and MySQL/MariaDB emit warnings. Doing the structured shape now — while there's exactly one producer — means those connectors can populate the same type later without a breaking interface change.

Changes

  • src/connectors/interface.ts: add DatabaseMessage { text; severity?; code?; line? }; SQLResult.messages is now DatabaseMessage[]. Only text is guaranteed — the rest are populated when the driver exposes them.
  • src/connectors/sqlserver/index.ts: map the info event into DatabaseMessage (text/severity/code/line) instead of pushing the raw string.
  • src/tools/execute-sql.ts: unchanged — it already forwards result.messages as-is, so clients now receive the structured objects automatically.
  • src/connectors/__tests__/sqlserver.integration.test.ts: assertions updated to read .text.

Notes

  • Backward-compatible at the wire level only in spirit — the messages field shape changes from string[] to objects, but feat(sqlserver): surface informational messages (STATISTICS, PRINT) in query results #307 has not shipped in a release yet, so no consumers depend on the old form.
  • severity is intentionally a general string (PG level name vs. SQL Server numeric class as a string), documented per-engine, rather than a SQL-Server-specific field name.

Verification

  • npx tsc --noEmit: zero new errors (135 pre-existing on both main and this branch; CI builds via tsup).
  • pnpm run build: ✓
  • Unit tests: 668/668 pass.
  • SQL Server integration tests require Docker and were not run here.

🤖 Generated with Claude Code

Promote the informational-messages field added in #307 from a bare
string[] to a structured DatabaseMessage[] so severity, code, and line
are preserved instead of discarded. This is a cross-database shape:
PostgreSQL NOTICE/WARNING and MySQL warnings can populate the same type
later without a breaking interface change.

- interface.ts: add DatabaseMessage type; SQLResult.messages is now
  DatabaseMessage[]
- sqlserver: map the mssql `info` event (message/class/number/lineNumber)
  into DatabaseMessage; severity lets clients distinguish PRINT from warnings
- execute_sql forwards the structured array unchanged
- update SQL Server integration tests to assert on `.text`

Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Copilot AI review requested due to automatic review settings June 7, 2026 14:27

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.

Pull request overview

This PR refactors SQLResult.messages from a lossy string[] into a structured DatabaseMessage[] so database informational output can carry per-message metadata (severity/code/line) rather than being flattened to plain text.

Changes:

  • Introduces DatabaseMessage in the connector interface and updates SQLResult.messages to DatabaseMessage[].
  • Updates the SQL Server connector to map mssql info events into { text, severity, code, line }.
  • Adjusts SQL Server integration tests to assert on msg.text instead of raw string messages.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File Description
src/connectors/interface.ts Adds DatabaseMessage type and updates SQLResult.messages to use it.
src/connectors/sqlserver/index.ts Collects SQL Server info events into structured DatabaseMessage objects.
src/connectors/tests/sqlserver.integration.test.ts Updates assertions to use DatabaseMessage.text.

@tianzhou tianzhou merged commit 587db27 into main Jun 7, 2026
3 checks passed
@tianzhou tianzhou deleted the feat/structured-db-messages branch June 9, 2026 10:42
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.

2 participants