Skip to content

fix(mcp): run SQL Server scripts from MCP, AppleScript and the assistant as GO batches and return every result set - #3123

Merged
datlechin merged 2 commits into
mainfrom
fix/mcp-sql-server-batches
Sep 24, 2026
Merged

datlechin merged 2 commits into
mainfrom
fix/mcp-sql-server-batches

Conversation

@datlechin

@datlechin datlechin commented Sep 24, 2026 •

Copy link
Copy Markdown
Member

What was wrong

#3105 taught the query editor to send a SQL Server script as GO batches through executeBatch and show every result set. The other three ways SQL reaches a driver never learned it. MCP execute_query, the AI assistant's execute_query and AppleScript run query all run through DatabaseAccessBridge.runStatement, which knows only the one-result call (executeUserQuery / execute). On SQL Server that gave three wrong answers:

  • A script without semicolons (DECLARE @sn ... SELECT ... SELECT ...) or a procedure call (EXEC sp_help 'dbo.orders') passed the gate as one statement and came back with only its first result set, plus the driver's note "End each statement with a semicolon to see every result".
  • Following that note got the call refused: the external gate splits at ; and GO and answers "Send one statement at a time." So the SQL Server batch variables are lost between statements #3078 reporter's script, with its semicolons, could not be sent at all, and a variable cannot be declared in one call and read in the next, because it lives only in its batch.
  • A GO line before a lone statement reached the server, which answers Msg 2812: Could not find stored procedure 'GO'., and GO 3 after one ran it once. Measured: SQLStatementScanner.executableText, which the old path sent, returns "GO\nSELECT 1 AS one" unchanged and "SELECT 1" for "SELECT 1\nGO 3", and Azure SQL Edge answers the first with Msg 2812.

What changed

  • DatabaseAccessBridge.runScript takes the route the editor takes for the same text on the same connection: QueryBatchPlanner cuts it at GO, and QueryExecutionRoute.resolve picks the path. A lone plain query keeps the bounded single path. Anything else, on a driver that sends batches whole, runs batch by batch on one lease under the same timeout race and cancellation as a statement. GO n repeats a batch, and every result set is held to max_rows. A batch that raises an error fails the call with the editor's own message (Batch 2/3 failed: Line 7: ... plus what stays applied) and stops the script. The session is asked before and after, as the editor asks, so a transaction the script left open is reported. A driver without resultSetBatches (an older SQL Server plugin) refuses a multi-statement script or GO n instead of running part of it.
  • ExternalStatementGate.acceptsScripts(on:): a caller that takes scripts may send several statements in one call only where the engine cuts scripts into batches at GO lines. That is SQL Server's grammar today; every other engine keeps one statement per call. The gate still classifies the whole script by its worst statement, so one write needs tools:write / External Clients Read & Write / Safe Mode, and one DROP is refused. The gate runs before connecting, so it decides from the dialect; the driver capability is checked at run time.
  • MCP execute_query and the assistant's execute_query run through it (ToolQueryExecutor with unit: .script). The response is additive: the existing fields still describe one result, the first, with rows_affected for the whole script and any notice in status_message. result_sets[] lists every result set (columns, rows, row_count, is_truncated) only when there is more than one, so a single SELECT's payload is byte-for-byte what it was. The output schema declares it. History and the audit log count rows across every result set.
  • AppleScript run query runs through it too. The query result record keeps its meaning and gains results, a list of the new result set record, empty unless a script returned more than one.
  • Shared pieces the editor now uses as well, so the two paths cannot drift: QueryBatchResult.followed(by:) (repetitions, the 100 result set ceiling, error positions), DatabaseDriver.answerBatch (the nil fallback), BatchRunNotice, QueryBatchPlanner.batches(in:model:grammar:) and QueryExecutionRoute.resolve(_:databaseType:sendsBatchesWhole:). The editor's behaviour is unchanged.
  • Other tools that run generated SQL through the bridge (browse_table, export_data, maintenance, server tools, confirm_destructive_operation, explain_query) keep the single-statement path.

The AppleScript names were measured, not chosen. A throwaway scriptable app with the same sdef shapes, running its AppleScript against itself (osascript would stop at the Automation consent prompt): a property named result sets beside a record type result set compiles, then AppleScript reads it as "every result set of r" and answers {}. results works for item access, repeat with and set x to results of r. Inside a tell block count of results of r answers 0 because count goes to the app, so the docs assign first. ScriptingDictionaryTests now refuses a property named the plural of a record type.

Verification

  • verify.sh generate: PASS. verify.sh build: PASS, before and after rebasing onto main. verify.sh build MSSQLDriver: PASS.
  • verify.sh lint on all 24 changed Swift files: 0 violations.
  • verify.sh docs: PASS. check-writing-style.sh and check-docs-against-source.py: clean.
  • scripts/ci/check-ios-shared-isolation.py: clean (no shared file touched).
  • Tests added. Not yet run locally: every xcodebuild test on this machine since 21:33 ends INCONCLUSIVE with "The test runner hung before establishing connection" and 0 cases executed, including a run with no other xcodebuild on the machine (testmanagerd wedged). The test target with these suites builds with 0 errors, before and after the rebase onto main: each run got as far as launching the test host. PR CI is the first run of these suites:
    • DatabaseAccessBridgeScriptTests (bridge with a fake batch-capable driver injected into DatabaseManager): the SQL Server batch variables are lost between statements #3078 script goes out as one batch with the row cap and all four result sets come back; EXEC sp_help returns every result set; GO and GO 3 cut and repeat batches and never reach the server; GO before a lone SELECT is not sent and the SELECT keeps the bounded path; totals and an open-transaction notice land on the first result; a batch error fails the call as Batch 2/3 failed: Line 3: ... and the third batch is never sent; result sets past the ceiling are counted; a driver without batches refuses a script and GO 2.
    • MCPScriptResultTests: result_sets and the top-level fields, the single-result payload equal to the old encoder's, the open transaction notice, the output schema, and ToolQueryExecutor with unit: .script end to end over an injected session.
    • QueryBatchResultTests, ExternalStatementGateTests (which engines take scripts; a SQL Server script is still refused on a read-only connection and with a DROP in it), ScriptResultEncoderTests (results), ScriptingDictionaryTests (the result set keys; no property named a record type's plural).
  • Live, against Azure SQL Edge 15 (tp-mssql-3078, own database pr_mcp_sql_server_batches): MSSQL_SA_PASSWORD=... scripts/check-mssql-batch-results.sh 127.0.0.1 14339 sa pr_mcp_sql_server_batches, 52/52 checks pass, twice. That is the driver path this change sends every external script through, including the reporter's script as one batch with four result sets.

Not covered

  • The bridge could not be driven live without the UI. A sandboxed Debug app never starts the MCP server (PostLaunchWork returns early when storage is isolated) and only the Settings pane starts it; AppleScript needs Automation consent. The bridge is covered by the unit tests above over a fake driver, and the server side by the live check.
  • No UI test: the change has no UI, and SQL Server cannot run in the UI test sandbox.
  • PRINT output of a script is not returned to external callers, as it was not for a statement before.

Found while fixing #3078 (#3105).

…ant as GO batches and return every result set
@mintlify

mintlify Bot commented Sep 24, 2026 •

Copy link
Copy Markdown

Preview deployment for your docs. Learn more about Mintlify Previews.

Project Status Preview Updated
TablePro 🟢 Ready View Preview Sep 24, 2026, 4:15 PM

💡 Tip: Enable Automations to automatically generate PRs for you.

Signed-off-by: Ngô Quốc Đạt <datlechin@gmail.com>
@datlechin
datlechin merged commit 456bfc7 into main Sep 24, 2026
4 checks passed
@datlechin
datlechin deleted the fix/mcp-sql-server-batches branch September 24, 2026 16:15
@datlechin

Copy link
Copy Markdown
Member Author

Follow-up in #3124. Review found that the execution gate still refused any SQL Server script of more than one statement on all three entry points, because no caller claimed .mayRunMultiStatement. It also found that a leading GO line still reached the server through the tools that send one statement. #3124 fixes both.

This branch was successfully deployed

1 active deployment
staging - docs — ac6babe8 Deployed Sep 24, 2026 by mintlify[bot]
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