Skip to content

feat(mcp): add common DDL and SQL formatting utilities - #965

Merged
mk3008 merged 2 commits into
mainfrom
codex/mcp-common-io-phase1
Aug 11, 2026
Merged

feat(mcp): add common DDL and SQL formatting utilities#965
mk3008 merged 2 commits into
mainfrom
codex/mcp-common-io-phase1

Conversation

@mk3008

@mk3008 mk3008 commented Aug 11, 2026

Copy link
Copy Markdown
Owner

Summary

  • Add core parseSqlFormatterOptions as the single strict trust boundary for JSON, MCP, and config formatter options.
  • Expose typed validation codes for non-object input, unknown options, and invalid values without changing SqlFormatter constructor behavior.
  • Remove the duplicated formatter option specification from @rawsql-ts/mcp-server while preserving config < inline precedence and MCP error codes.
  • Raise path-backed DDL maxFileBytes from 1 MiB to 50 MiB while keeping inline DDL at 1 MiB, total input at 50 MiB, and maxFiles at 5,000.
  • Typecheck every MCP Vitest file and keep all six existing MCP tool schemas and outputs unchanged.

Verification

  • rawsql-ts full tests: 243 files passed; 2,529 tests passed and 1 skipped.
  • @rawsql-ts/mcp-server tests and test typecheck: 2 files and 41 tests passed.
  • @rawsql-ts/investigation-core: 76 tests passed; @rawsql-ts/sql-grep-core: 9 tests passed.
  • Core and MCP lint/build plus root typecheck passed.
  • API output-shape guard and git diff --check passed.
  • Pre-commit full workspace: 266 files passed; 2,767 tests passed and 1 environment-dependent Postgres test skipped; workspace build and lint passed.
  • Core parser benchmark and complex SQL formatter demo completed successfully.
  • Existing exact six-tool MCP schema/output regression passed.

Merge Readiness

  • No baseline exception requested.
  • Baseline exception requested and linked below.

Tracking issue: not needed; no baseline exception requested.
Scoped checks run: not needed; no baseline exception requested.
Why full baseline is not required: full baseline exception path not used for this PR.

Self Review

Self-review workflow: Two-cycle self-review skill after scoped and full-workspace verification.
Self-review result: No scoped merge blockers remain; the acknowledged pathname TOCTOU hardening is intentionally deferred per the review-fix scope.
Concept-review workflow: Reviewed root and core AGENTS.md/DESIGN.md, core public API ownership, Changeset policy, and MCP package boundary.
Concept-review result: Formatter runtime validation now has one core authority; no package-boundary or existing MCP I/O violation remains.

CLI Surface Migration

  • No migration packet required for this CLI change.
  • CLI/user-facing surface change and migration packet completed.

No-migration rationale: not selected for this PR.
Upgrade note: not selected for this PR.
Deprecation/removal plan or issue: not selected for this PR.
Docs/help/examples updated: not selected for this PR.
Release/changeset wording: not selected for this PR.

Scaffold Contract Proof

  • No scaffold contract proof required for this PR.
  • Scaffold contract proof completed.

No-proof rationale: not selected for this PR.
Non-edit assertion: not selected for this PR.
Fail-fast input-contract proof: not selected for this PR.
Generated-output viability proof: not selected for this PR.

Summary by CodeRabbit

  • New Features
    • Added support for loading DDL from inline SQL, individual workspace files, and directories with recursive .sql discovery.
    • Added configurable safeguards for source size, file count, and total input size.
    • Added optional formatting for generated SQL using configuration files or inline options.
    • Added strict validation for SQL formatter options, with clear errors for unsupported or invalid settings.
  • Bug Fixes
    • Improved workspace path validation to prevent traversal and symlink escapes.
    • Ensured deterministic source ordering and duplicate handling.

@coderabbitai

coderabbitai Bot commented Aug 11, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

The MCP server now validates canonical workspace paths, resolves inline and workspace DDL sources, formats explicit generated SQL artifacts, exposes typed input errors, and type-checks tests before running Vitest. The core package also exposes strict SQL formatter-option parsing.

Changes

MCP input processing and formatter validation

Layer / File(s) Summary
Strict formatter-option parsing
packages/core/src/transformers/SqlFormatterOptionsParser.ts, packages/core/src/index.ts, packages/core/tests/transformers/SqlFormatterOptionsParser.test.ts, .changeset/tidy-format-options.md
The core package validates external formatter options, rejects unsupported shapes and keys, copies accepted values, exposes structured errors, and adds a minor-release changeset.
Workspace validation and shared errors
packages/mcp-server/src/inputError.ts, packages/mcp-server/src/workspacePaths.ts, packages/mcp-server/src/server.ts
The server uses shared input errors and canonical workspace path utilities. Path resolution rejects traversal, symlink escapes, invalid filesystem types, and paths outside the workspace.
DDL source resolution
packages/mcp-server/src/ddlSources.ts, packages/mcp-server/tests/commonIo.test.ts
DDL resolution accepts inline SQL and workspace files or directories. It validates content, discovers and sorts SQL files, removes duplicates, preserves source identity, and enforces count and size limits.
Generated SQL formatting
packages/mcp-server/src/sqlFormatting.ts, packages/mcp-server/tests/commonIo.test.ts
Formatter configuration supports workspace-relative JSON files and inline options. Explicit generated SQL artifacts are formatted while metadata is preserved. Invalid options, artifacts, and formatting operations produce McpInputError results.
Test type-checking workflow
packages/mcp-server/tsconfig.tests.json, packages/mcp-server/package.json, packages/mcp-server/tests/server.test.ts
The package now type-checks the test project before running Vitest. The test configuration uses no-emission compiler settings and explicit source and test inclusions.

Estimated code review effort: 4 (Complex) | ~45 minutes

Sequence Diagram(s)

sequenceDiagram
  participant Caller
  participant resolveSqlFormatting
  participant resolveWorkspacePath
  participant parseSqlFormatterOptions
  participant formatGeneratedSqlArtifact
  Caller->>resolveSqlFormatting: formatter input
  resolveSqlFormatting->>resolveWorkspacePath: workspace-relative JSON config path
  resolveSqlFormatting->>parseSqlFormatterOptions: merged formatter options
  resolveSqlFormatting-->>Caller: resolved formatter options
  Caller->>formatGeneratedSqlArtifact: generated SQL artifact
  formatGeneratedSqlArtifact-->>Caller: formatted artifact or McpInputError
Loading

Possibly related PRs

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 31.25% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Title check ✅ Passed The title clearly summarizes the main MCP changes by naming the added DDL and SQL formatting utilities.
Description check ✅ Passed The description covers the template sections, implementation scope, verification results, and review status, but leaves migration and scaffold checkboxes unselected.
✨ Finishing Touches 💡 1
📝 Generate docstrings 💡
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/mcp-common-io-phase1

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Actionable comments posted: 3

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@packages/mcp-server/src/sqlFormatting.ts`:
- Around line 161-170: Validate formatter option keys as own properties before
retrieving validators in the formatter-options validation loop of
sqlFormatting.ts, so toString, constructor, and __proto__ produce
FORMAT_OPTION_UNKNOWN without errors. Add tests covering all three unknown keys
in packages/mcp-server/tests/commonIo.test.ts at lines 244-256.

In `@packages/mcp-server/src/workspacePaths.ts`:
- Around line 53-74: Update resolveWorkspacePath in
packages/mcp-server/src/workspacePaths.ts lines 53-74 to return a stable
validated file descriptor or equivalent identifier instead of only a pathname.
Update packages/mcp-server/src/ddlSources.ts lines 79-85 and
packages/mcp-server/src/sqlFormatting.ts lines 143-149 to perform size checks
and reads through that same identifier, avoiding subsequent pathname-based
statSync, readFileSync, or JSON reads.

In `@packages/mcp-server/tsconfig.tests.json`:
- Line 9: Update the include configuration in tsconfig.tests.json to cover all
Vitest test files by specifying tests/**/*.test.ts, ensuring files such as
tests/server.test.ts are included in type checking while preserving the existing
source inclusion.
🪄 Autofix

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: defaults

Review profile: CHILL

Plan: Pro Plus

Run ID: 79bcccbe-e1f6-4fc2-b825-03709a14e22f

📥 Commits

Reviewing files that changed from the base of the PR and between c2f3458 and 5673e6e.

📒 Files selected for processing (8)
  • packages/mcp-server/package.json
  • packages/mcp-server/src/ddlSources.ts
  • packages/mcp-server/src/inputError.ts
  • packages/mcp-server/src/server.ts
  • packages/mcp-server/src/sqlFormatting.ts
  • packages/mcp-server/src/workspacePaths.ts
  • packages/mcp-server/tests/commonIo.test.ts
  • packages/mcp-server/tsconfig.tests.json

Comment thread packages/mcp-server/src/sqlFormatting.ts Outdated
Comment thread packages/mcp-server/src/workspacePaths.ts
Comment thread packages/mcp-server/tsconfig.tests.json Outdated
@mk3008
mk3008 merged commit b5a0c2b into main Aug 11, 2026
12 checks passed
@mk3008
mk3008 deleted the codex/mcp-common-io-phase1 branch August 11, 2026 08:33
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.

1 participant