From c973807ff80df2cf9c66d92b87e8c89063768c26 Mon Sep 17 00:00:00 2001 From: Ngo Quoc Dat Date: Tue, 4 Aug 2026 01:43:40 +0700 Subject: [PATCH] fix(mcp): honor the configured row limits in export_data (#2012) --- CHANGELOG.md | 4 + .../AI/Chat/ChatToolArgumentDecoder.swift | 12 +-- .../ConfirmDestructiveOperationChatTool.swift | 2 +- .../AI/Chat/Tools/ExecuteQueryChatTool.swift | 24 ++--- .../ConfirmDestructiveOperationTool.swift | 4 +- .../MCP/Protocol/Tools/ExecuteQueryTool.swift | 26 +++--- .../MCP/Protocol/Tools/ExportDataTool.swift | 72 +++++++++++--- .../Protocol/Tools/MCPArgumentDecoder.swift | 4 +- .../MCP/Protocol/Tools/MCPLimitResolver.swift | 13 +++ .../MCP/Protocol/Tools/MCPToolServices.swift | 14 +++ .../Infrastructure/SettingsValidation.swift | 3 + .../Core/Storage/AppSettingsManager.swift | 13 ++- TablePro/Models/Settings/MCPSettings.swift | 24 +++++ .../AI/ChatToolArgumentDecoderTests.swift | 25 +++++ TableProTests/Core/MCP/MCPSettingsTests.swift | 49 ++++++++++ .../Protocol/Tools/ExportDataToolTests.swift | 91 ++++++++++++++++++ .../Tools/MCPLimitResolverTests.swift | 93 +++++++++++++++++++ docs/customization/settings.mdx | 4 +- docs/external-api/mcp-tools.mdx | 6 +- 19 files changed, 420 insertions(+), 63 deletions(-) create mode 100644 TablePro/Core/MCP/Protocol/Tools/MCPLimitResolver.swift create mode 100644 TableProTests/Core/MCP/Protocol/Tools/MCPLimitResolverTests.swift diff --git a/CHANGELOG.md b/CHANGELOG.md index 69dcafa3b..3d642d4ee 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,6 +12,10 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0 - Saving on a MySQL or MariaDB server that starts sessions read-only no longer fails with "Cannot execute statement in a READ ONLY transaction". TablePro marks a transaction read-write before it writes instead of inheriting the server default. Same for PostgreSQL, CockroachDB, and Redshift. (#2009) - Changing Safe Mode in the connection form now applies to an open connection instead of waiting for a reconnect. (#2009) - A read-only error now says whether the database server or Safe Mode refused the write. (#2009) +- The MCP server's **Default row limit** and **Maximum row limit** now apply to the `export_data` tool, so raising the maximum really does export more rows. Export previously ignored both and used a fixed 50,000, so an export with no `max_rows` now returns the default row limit (500) until you raise it. Export also honours the configured query timeout, reports whether the limit clipped the result, and appears in query history and the activity log like other MCP queries. (#2012) +- Exporting a table over MCP no longer fails on SQL Server, Oracle, and Teradata. The row limit is now written in each database's own syntax instead of always using `LIMIT`. (#2012) +- Setting the MCP server's row limits or query timeout to zero or a negative number no longer crashes the app on the next tool call. Values outside the supported range are corrected as you type. (#2012) +- AI chat no longer crashes when the model asks a tool for a row limit or timeout larger than TablePro can count to. The tool now falls back to the configured limit. (#2012) ## [0.62.0] - 2026-08-02 diff --git a/TablePro/Core/AI/Chat/ChatToolArgumentDecoder.swift b/TablePro/Core/AI/Chat/ChatToolArgumentDecoder.swift index 1fe9b145c..5d0f903cc 100644 --- a/TablePro/Core/AI/Chat/ChatToolArgumentDecoder.swift +++ b/TablePro/Core/AI/Chat/ChatToolArgumentDecoder.swift @@ -41,17 +41,17 @@ enum ChatToolArgumentDecoder { static func optionalInt( _ args: JsonValue, key: String, - default fallback: Int, + default fallback: Int? = nil, clamp: ClosedRange? = nil ) -> Int? { guard case .object(let dict) = args, let value = dict[key] else { return fallback } - let raw: Int? + let decoded: Int? switch value { - case .int(let int): raw = int - case .double(let double): raw = Int(double) - default: raw = nil + case .int(let int): decoded = int + case .double(let double): decoded = Int(exactly: double.rounded(.towardZero)) + default: decoded = nil } - guard let raw else { return fallback } + guard let raw = decoded ?? fallback else { return nil } if let clamp { return max(clamp.lowerBound, min(raw, clamp.upperBound)) } return raw } diff --git a/TablePro/Core/AI/Chat/Tools/ConfirmDestructiveOperationChatTool.swift b/TablePro/Core/AI/Chat/Tools/ConfirmDestructiveOperationChatTool.swift index 4822aadde..cfe7cd7c4 100644 --- a/TablePro/Core/AI/Chat/Tools/ConfirmDestructiveOperationChatTool.swift +++ b/TablePro/Core/AI/Chat/Tools/ConfirmDestructiveOperationChatTool.swift @@ -68,7 +68,7 @@ struct ConfirmDestructiveOperationChatTool: ChatTool { connectionId: connectionId, databaseName: meta.databaseName, maxRows: 0, - timeoutSeconds: mcpSettings.queryTimeoutSeconds, + timeoutSeconds: MCPLimitResolver.resolveTimeoutSeconds(requested: nil, settings: mcpSettings), principalLabel: String(localized: "AI Chat") ) return ChatToolResult(content: payload.jsonString(prettyPrinted: true)) diff --git a/TablePro/Core/AI/Chat/Tools/ExecuteQueryChatTool.swift b/TablePro/Core/AI/Chat/Tools/ExecuteQueryChatTool.swift index 88f2ffb66..510b4e20e 100644 --- a/TablePro/Core/AI/Chat/Tools/ExecuteQueryChatTool.swift +++ b/TablePro/Core/AI/Chat/Tools/ExecuteQueryChatTool.swift @@ -17,11 +17,11 @@ struct ExecuteQueryChatTool: ChatTool { "connection_id": ChatToolSchemaBuilder.connectionId, "query": ChatToolSchemaBuilder.string(description: "SQL or NoSQL query text"), "max_rows": ChatToolSchemaBuilder.integer( - description: "Maximum rows to return (default 500, max 10000). Pass null to use default.", + description: "Maximum rows to return, capped at the server's configured maximum row limit. Pass null to use the configured default row limit.", optional: true ), "timeout_seconds": ChatToolSchemaBuilder.integer( - description: "Query timeout in seconds (default 30, max 300). Pass null to use default.", + description: "Query timeout in seconds (max 300). Pass null to use the server's configured query timeout.", optional: true ), "database": ChatToolSchemaBuilder.string( @@ -56,18 +56,14 @@ struct ExecuteQueryChatTool: ChatTool { } let mcpSettings = await MainActor.run { AppSettingsManager.shared.mcp } - let maxRows = ChatToolArgumentDecoder.optionalInt( - input, - key: "max_rows", - default: mcpSettings.defaultRowLimit, - clamp: 1...mcpSettings.maxRowLimit - ) ?? mcpSettings.defaultRowLimit - let timeoutSeconds = ChatToolArgumentDecoder.optionalInt( - input, - key: "timeout_seconds", - default: mcpSettings.queryTimeoutSeconds, - clamp: 1...300 - ) ?? mcpSettings.queryTimeoutSeconds + let maxRows = MCPLimitResolver.resolveMaxRows( + requested: ChatToolArgumentDecoder.optionalInt(input, key: "max_rows"), + settings: mcpSettings + ) + let timeoutSeconds = MCPLimitResolver.resolveTimeoutSeconds( + requested: ChatToolArgumentDecoder.optionalInt(input, key: "timeout_seconds"), + settings: mcpSettings + ) let tier = QueryClassifier.classifyTier(query, databaseType: meta.databaseType) if tier == .destructive { diff --git a/TablePro/Core/MCP/Protocol/Tools/ConfirmDestructiveOperationTool.swift b/TablePro/Core/MCP/Protocol/Tools/ConfirmDestructiveOperationTool.swift index cb09a1ccf..650810fd1 100644 --- a/TablePro/Core/MCP/Protocol/Tools/ConfirmDestructiveOperationTool.swift +++ b/TablePro/Core/MCP/Protocol/Tools/ConfirmDestructiveOperationTool.swift @@ -79,8 +79,8 @@ public struct ConfirmDestructiveOperationTool: MCPToolImplementation { capabilities: [.mayWrite, .mayRunDestructive, .confirmationPreCleared] ) - let mcpSettings = await MainActor.run { AppSettingsManager.shared.mcp } - let timeoutSeconds = mcpSettings.queryTimeoutSeconds + let mcpSettings = await services.settingsProvider() + let timeoutSeconds = MCPLimitResolver.resolveTimeoutSeconds(requested: nil, settings: mcpSettings) Self.logger.debug("confirm_destructive_operation invoked for connection \(connectionId.uuidString, privacy: .public)") diff --git a/TablePro/Core/MCP/Protocol/Tools/ExecuteQueryTool.swift b/TablePro/Core/MCP/Protocol/Tools/ExecuteQueryTool.swift index 93ceb6091..054fe0fef 100644 --- a/TablePro/Core/MCP/Protocol/Tools/ExecuteQueryTool.swift +++ b/TablePro/Core/MCP/Protocol/Tools/ExecuteQueryTool.swift @@ -19,11 +19,11 @@ public struct ExecuteQueryTool: MCPToolImplementation { ]), "max_rows": .object([ "type": .string("integer"), - "description": .string(String(localized: "Maximum rows to return (default 500, max 10000)")) + "description": .string(String(localized: "Maximum rows to return. Defaults to the server's configured default row limit and is capped at its maximum row limit.")) ]), "timeout_seconds": .object([ "type": .string("integer"), - "description": .string(String(localized: "Query timeout in seconds (default 30, max 300)")) + "description": .string(String(localized: "Query timeout in seconds (max 300). Defaults to the server's configured query timeout.")) ]), "database": .object([ "type": .string("string"), @@ -57,19 +57,15 @@ public struct ExecuteQueryTool: MCPToolImplementation { let connectionId = try MCPArgumentDecoder.requireUuid(arguments, key: "connection_id") let query = try MCPArgumentDecoder.requireString(arguments, key: "query") - let mcpSettings = await MainActor.run { AppSettingsManager.shared.mcp } - let maxRows = MCPArgumentDecoder.optionalInt( - arguments, - key: "max_rows", - default: mcpSettings.defaultRowLimit, - clamp: 1...mcpSettings.maxRowLimit - ) ?? mcpSettings.defaultRowLimit - let timeoutSeconds = MCPArgumentDecoder.optionalInt( - arguments, - key: "timeout_seconds", - default: mcpSettings.queryTimeoutSeconds, - clamp: 1...300 - ) ?? mcpSettings.queryTimeoutSeconds + let mcpSettings = await services.settingsProvider() + let maxRows = MCPLimitResolver.resolveMaxRows( + requested: MCPArgumentDecoder.optionalInt(arguments, key: "max_rows"), + settings: mcpSettings + ) + let timeoutSeconds = MCPLimitResolver.resolveTimeoutSeconds( + requested: MCPArgumentDecoder.optionalInt(arguments, key: "timeout_seconds"), + settings: mcpSettings + ) let database = MCPArgumentDecoder.optionalString(arguments, key: "database") let schema = MCPArgumentDecoder.optionalString(arguments, key: "schema") diff --git a/TablePro/Core/MCP/Protocol/Tools/ExportDataTool.swift b/TablePro/Core/MCP/Protocol/Tools/ExportDataTool.swift index 61a736764..37fa1c534 100644 --- a/TablePro/Core/MCP/Protocol/Tools/ExportDataTool.swift +++ b/TablePro/Core/MCP/Protocol/Tools/ExportDataTool.swift @@ -1,5 +1,6 @@ import Foundation import os +import TableProPluginKit public struct ExportDataTool: MCPToolImplementation { public static let name = "export_data" @@ -33,7 +34,7 @@ public struct ExportDataTool: MCPToolImplementation { ]), "max_rows": .object([ "type": .string("integer"), - "description": .string(String(localized: "Maximum rows to export (default 50000)")) + "description": .string(String(localized: "Maximum rows to export. Defaults to the server's configured default row limit and is capped at its maximum row limit.")) ]) ]), "required": .array([.string("connection_id"), .string("format")]) @@ -63,12 +64,13 @@ public struct ExportDataTool: MCPToolImplementation { let query = MCPArgumentDecoder.optionalString(arguments, key: "query") let tables = MCPArgumentDecoder.optionalStringArray(arguments, key: "tables") let outputPath = MCPArgumentDecoder.optionalString(arguments, key: "output_path") - let maxRows = MCPArgumentDecoder.optionalInt( - arguments, - key: "max_rows", - default: 50_000, - clamp: 1...100_000 - ) ?? 50_000 + let settings = await services.settingsProvider() + let maxRows = MCPLimitResolver.resolveMaxRows( + requested: MCPArgumentDecoder.optionalInt(arguments, key: "max_rows"), + settings: settings + ) + let timeoutSeconds = MCPLimitResolver.resolveTimeoutSeconds(requested: nil, settings: settings) + let fetchLimit = maxRows + 1 guard Self.allowedFormats.contains(format) else { throw MCPProtocolError.invalidParams( @@ -103,9 +105,10 @@ public struct ExportDataTool: MCPToolImplementation { queries.append((label: "query", sql: query)) } else if let tables { let quoteIdentifier = Self.identifierQuoter(for: meta.databaseType) + let autoLimitStyle = Self.autoLimitStyle(for: meta.databaseType) for table in tables { let quoted = try Self.quoteQualifiedIdentifier(table, quoter: quoteIdentifier) - let sql = "SELECT * FROM \(quoted) LIMIT \(maxRows)" + let sql = Self.limitedSelectAll(from: quoted, limit: fetchLimit, autoLimitStyle: autoLimitStyle) try await services.authPolicy.checkSafeModeDialog( sql: sql, connectionId: connectionId, @@ -118,21 +121,32 @@ public struct ExportDataTool: MCPToolImplementation { var exportResults: [JsonValue] = [] var totalRowsExported = 0 + var anyTruncated = false for (label, sql) in queries { - let result = try await services.connectionBridge.executeQuery( - connectionId: connectionId, + let result = try await ToolQueryExecutor.executeAndLog( + services: services, query: sql, - maxRows: maxRows, - timeoutSeconds: 60 + connectionId: connectionId, + databaseName: meta.databaseName, + maxRows: fetchLimit, + timeoutSeconds: timeoutSeconds, + principalLabel: context.principal.metadata.label ) guard let columns = result["columns"]?.arrayValue, - let rows = result["rows"]?.arrayValue + let fetched = result["rows"]?.arrayValue else { throw MCPProtocolError.internalError(detail: "Unexpected query result structure") } + let limited = Self.applyRowLimit( + to: fetched, + maxRows: maxRows, + driverReportedTruncation: result["is_truncated"]?.boolValue ?? false + ) + let rows = limited.rows + let isTruncated = limited.isTruncated let columnNames = columns.compactMap(\.stringValue) let formatted: String @@ -148,11 +162,13 @@ public struct ExportDataTool: MCPToolImplementation { } totalRowsExported += rows.count + anyTruncated = anyTruncated || isTruncated exportResults.append(.object([ "label": .string(label), "format": .string(format), - "row_count": result["row_count"] ?? .int(0), + "row_count": .int(rows.count), + "is_truncated": .bool(isTruncated), "data": .string(formatted) ])) } @@ -173,7 +189,8 @@ public struct ExportDataTool: MCPToolImplementation { let response: JsonValue = .object([ "path": .string(fileURL.path), - "rows_exported": .int(totalRowsExported) + "rows_exported": .int(totalRowsExported), + "is_truncated": .bool(anyTruncated) ]) return .structured(response) } @@ -195,6 +212,31 @@ public struct ExportDataTool: MCPToolImplementation { } } + static func applyRowLimit( + to fetched: [JsonValue], + maxRows: Int, + driverReportedTruncation: Bool + ) -> (rows: [JsonValue], isTruncated: Bool) { + (Array(fetched.prefix(maxRows)), fetched.count > maxRows || driverReportedTruncation) + } + + static func autoLimitStyle(for databaseType: DatabaseType) -> AutoLimitStyle { + (try? resolveSQLDialect(for: databaseType))?.autoLimitStyle ?? .limit + } + + static func limitedSelectAll(from quotedTable: String, limit: Int, autoLimitStyle: AutoLimitStyle) -> String { + switch autoLimitStyle { + case .top: + return "SELECT TOP \(limit) * FROM \(quotedTable)" + case .fetchFirst: + return "SELECT * FROM \(quotedTable) FETCH FIRST \(limit) ROWS ONLY" + case .none: + return "SELECT * FROM \(quotedTable)" + default: + return "SELECT * FROM \(quotedTable) LIMIT \(limit)" + } + } + static func identifierQuoter(for databaseType: DatabaseType) -> (String) -> String { if let dialect = try? resolveSQLDialect(for: databaseType) { return quoteIdentifierFromDialect(dialect) diff --git a/TablePro/Core/MCP/Protocol/Tools/MCPArgumentDecoder.swift b/TablePro/Core/MCP/Protocol/Tools/MCPArgumentDecoder.swift index c5fa6631c..c9774387a 100644 --- a/TablePro/Core/MCP/Protocol/Tools/MCPArgumentDecoder.swift +++ b/TablePro/Core/MCP/Protocol/Tools/MCPArgumentDecoder.swift @@ -42,8 +42,8 @@ enum MCPArgumentDecoder { default defaultValue: Int? = nil, clamp: ClosedRange? = nil ) -> Int? { - let raw = args[key]?.intValue - guard let raw else { return defaultValue } + let raw = args[key]?.intValue ?? defaultValue + guard let raw else { return nil } guard let clamp else { return raw } return min(max(raw, clamp.lowerBound), clamp.upperBound) } diff --git a/TablePro/Core/MCP/Protocol/Tools/MCPLimitResolver.swift b/TablePro/Core/MCP/Protocol/Tools/MCPLimitResolver.swift new file mode 100644 index 000000000..933029945 --- /dev/null +++ b/TablePro/Core/MCP/Protocol/Tools/MCPLimitResolver.swift @@ -0,0 +1,13 @@ +import Foundation + +enum MCPLimitResolver { + static func resolveMaxRows(requested: Int?, settings: MCPSettings) -> Int { + guard let requested else { return settings.effectiveDefaultRowLimit } + return requested.clamped(to: settings.requestableRowLimitRange) + } + + static func resolveTimeoutSeconds(requested: Int?, settings: MCPSettings) -> Int { + guard let requested else { return settings.validatedQueryTimeoutSeconds } + return requested.clamped(to: SettingsValidationRules.mcpQueryTimeoutRange) + } +} diff --git a/TablePro/Core/MCP/Protocol/Tools/MCPToolServices.swift b/TablePro/Core/MCP/Protocol/Tools/MCPToolServices.swift index 725f9f440..6fef2b39b 100644 --- a/TablePro/Core/MCP/Protocol/Tools/MCPToolServices.swift +++ b/TablePro/Core/MCP/Protocol/Tools/MCPToolServices.swift @@ -3,9 +3,23 @@ import Foundation public struct MCPToolServices: Sendable { public let connectionBridge: MCPConnectionBridge public let authPolicy: MCPAuthPolicy + let settingsProvider: @Sendable () async -> MCPSettings public init(connectionBridge: MCPConnectionBridge, authPolicy: MCPAuthPolicy) { + self.init( + connectionBridge: connectionBridge, + authPolicy: authPolicy, + settingsProvider: { await MainActor.run { AppSettingsManager.shared.mcp } } + ) + } + + init( + connectionBridge: MCPConnectionBridge, + authPolicy: MCPAuthPolicy, + settingsProvider: @escaping @Sendable () async -> MCPSettings + ) { self.connectionBridge = connectionBridge self.authPolicy = authPolicy + self.settingsProvider = settingsProvider } } diff --git a/TablePro/Core/Services/Infrastructure/SettingsValidation.swift b/TablePro/Core/Services/Infrastructure/SettingsValidation.swift index adb0b08e4..06deddb6b 100644 --- a/TablePro/Core/Services/Infrastructure/SettingsValidation.swift +++ b/TablePro/Core/Services/Infrastructure/SettingsValidation.swift @@ -95,4 +95,7 @@ enum SettingsValidationRules { static let defaultPageSizeRange = 10...100_000 static let queryResultRowCapRange: ClosedRange = 100...500_000 static let minNonNegative = 0 + + static let mcpRowLimitRange: ClosedRange = 1...500_000 + static let mcpQueryTimeoutRange: ClosedRange = 1...300 } diff --git a/TablePro/Core/Storage/AppSettingsManager.swift b/TablePro/Core/Storage/AppSettingsManager.swift index ec39c8d84..caa364db3 100644 --- a/TablePro/Core/Storage/AppSettingsManager.swift +++ b/TablePro/Core/Storage/AppSettingsManager.swift @@ -126,14 +126,21 @@ final class AppSettingsManager { var mcp: MCPSettings { didSet { guard !isValidating else { return } + var validated = mcp + validated.maxRowLimit = mcp.validatedMaxRowLimit + validated.defaultRowLimit = mcp.validatedDefaultRowLimit + validated.queryTimeoutSeconds = mcp.validatedQueryTimeoutSeconds + if validated.allowRemoteConnections, !validated.requireAuthentication { + validated.requireAuthentication = true + } - if mcp.allowRemoteConnections, !mcp.requireAuthentication { + if validated != mcp { isValidating = true - mcp.requireAuthentication = true + mcp = validated isValidating = false } - storage.saveMCP(mcp) + storage.saveMCP(validated) syncTracker.markDirty(.settings, id: "mcp") let enabledChanged = mcp.enabled != oldValue.enabled let portChanged = mcp.port != oldValue.port diff --git a/TablePro/Models/Settings/MCPSettings.swift b/TablePro/Models/Settings/MCPSettings.swift index b90bf1c96..c5d78febc 100644 --- a/TablePro/Models/Settings/MCPSettings.swift +++ b/TablePro/Models/Settings/MCPSettings.swift @@ -52,5 +52,29 @@ struct MCPSettings: Codable, Equatable { logQueriesInHistory = try container.decodeIfPresent(Bool.self, forKey: .logQueriesInHistory) ?? true requireAuthentication = try container.decodeIfPresent(Bool.self, forKey: .requireAuthentication) ?? true allowRemoteConnections = try container.decodeIfPresent(Bool.self, forKey: .allowRemoteConnections) ?? false + + maxRowLimit = validatedMaxRowLimit + defaultRowLimit = validatedDefaultRowLimit + queryTimeoutSeconds = validatedQueryTimeoutSeconds + } + + var validatedMaxRowLimit: Int { + maxRowLimit.clamped(to: SettingsValidationRules.mcpRowLimitRange) + } + + var validatedDefaultRowLimit: Int { + defaultRowLimit.clamped(to: SettingsValidationRules.mcpRowLimitRange) + } + + var validatedQueryTimeoutSeconds: Int { + queryTimeoutSeconds.clamped(to: SettingsValidationRules.mcpQueryTimeoutRange) + } + + var effectiveDefaultRowLimit: Int { + min(validatedDefaultRowLimit, validatedMaxRowLimit) + } + + var requestableRowLimitRange: ClosedRange { + SettingsValidationRules.mcpRowLimitRange.lowerBound...validatedMaxRowLimit } } diff --git a/TableProTests/Core/AI/ChatToolArgumentDecoderTests.swift b/TableProTests/Core/AI/ChatToolArgumentDecoderTests.swift index 4cf38960d..3e7213a2e 100644 --- a/TableProTests/Core/AI/ChatToolArgumentDecoderTests.swift +++ b/TableProTests/Core/AI/ChatToolArgumentDecoderTests.swift @@ -98,4 +98,29 @@ struct ChatToolArgumentDecoderTests { let args: JsonValue = .object(["max_rows": .string("ten")]) #expect(ChatToolArgumentDecoder.optionalInt(args, key: "max_rows", default: 500) == 500) } + + @Test("optionalInt falls back instead of trapping on a number wider than Int") + func optionalIntOutOfRangeDouble() { + let args: JsonValue = .object(["max_rows": .double(1e30)]) + #expect(ChatToolArgumentDecoder.optionalInt(args, key: "max_rows", default: 500) == 500) + } + + @Test("optionalInt falls back instead of trapping on a non-finite number") + func optionalIntNonFiniteDouble() { + #expect( + ChatToolArgumentDecoder.optionalInt( + .object(["max_rows": .double(.infinity)]), key: "max_rows", default: 500 + ) == 500 + ) + #expect( + ChatToolArgumentDecoder.optionalInt( + .object(["max_rows": .double(.nan)]), key: "max_rows", default: 500 + ) == 500 + ) + } + + @Test("optionalInt without a default returns nil when the key is absent") + func optionalIntNoDefaultMissingKey() { + #expect(ChatToolArgumentDecoder.optionalInt(.object([:]), key: "max_rows") == nil) + } } diff --git a/TableProTests/Core/MCP/MCPSettingsTests.swift b/TableProTests/Core/MCP/MCPSettingsTests.swift index 51241de6a..ed60cb8c2 100644 --- a/TableProTests/Core/MCP/MCPSettingsTests.swift +++ b/TableProTests/Core/MCP/MCPSettingsTests.swift @@ -25,6 +25,55 @@ struct MCPSettingsTests { #expect(!decoded.requireAuthentication) } + @Test("Row limits decode to the documented defaults") + func decodesRowLimitDefaults() throws { + let decoded = try JSONDecoder().decode(MCPSettings.self, from: Data("{}".utf8)) + #expect(decoded.defaultRowLimit == 500) + #expect(decoded.maxRowLimit == 10_000) + #expect(decoded.queryTimeoutSeconds == 30) + } + + @Test("A stored zero maximum row limit decodes to a usable value") + func decodesZeroMaxRowLimit() throws { + let json = Data(#"{"maxRowLimit": 0, "defaultRowLimit": 0, "queryTimeoutSeconds": 0}"#.utf8) + let decoded = try JSONDecoder().decode(MCPSettings.self, from: json) + #expect(decoded.maxRowLimit == 1) + #expect(decoded.defaultRowLimit == 1) + #expect(decoded.queryTimeoutSeconds == 1) + } + + @Test("Stored negative limits decode to usable values") + func decodesNegativeLimits() throws { + let json = Data(#"{"maxRowLimit": -100, "defaultRowLimit": -5, "queryTimeoutSeconds": -1}"#.utf8) + let decoded = try JSONDecoder().decode(MCPSettings.self, from: json) + #expect(decoded.maxRowLimit == 1) + #expect(decoded.defaultRowLimit == 1) + #expect(decoded.queryTimeoutSeconds == 1) + } + + @Test("A stored timeout above the protocol ceiling decodes clamped") + func decodesTimeoutAboveCeiling() throws { + let json = Data(#"{"queryTimeoutSeconds": 100000}"#.utf8) + let decoded = try JSONDecoder().decode(MCPSettings.self, from: json) + #expect(decoded.queryTimeoutSeconds == 300) + } + + @Test("A default row limit above the maximum stays stored but resolves capped") + func defaultAboveMaximumIsPreservedButCapped() throws { + let json = Data(#"{"maxRowLimit": 1000, "defaultRowLimit": 50000}"#.utf8) + let decoded = try JSONDecoder().decode(MCPSettings.self, from: json) + #expect(decoded.defaultRowLimit == 50_000) + #expect(decoded.effectiveDefaultRowLimit == 1_000) + } + + @Test("The requestable row limit range is never inverted") + func requestableRangeIsNeverInverted() { + for maxRowLimit in [-1, 0, 1, 10_000, 10_000_000] { + let range = MCPSettings(maxRowLimit: maxRowLimit).requestableRowLimitRange + #expect(range.lowerBound <= range.upperBound) + } + } + @Test("Default settings deny anonymous loopback without a token") func defaultDeniesAnonymousLoopback() async { let store = FakeMCPTokenStore() diff --git a/TableProTests/Core/MCP/Protocol/Tools/ExportDataToolTests.swift b/TableProTests/Core/MCP/Protocol/Tools/ExportDataToolTests.swift index 75d1eea07..b2d5e7b6e 100644 --- a/TableProTests/Core/MCP/Protocol/Tools/ExportDataToolTests.swift +++ b/TableProTests/Core/MCP/Protocol/Tools/ExportDataToolTests.swift @@ -3,8 +3,41 @@ import TableProPluginKit @testable import TablePro import Testing +private actor SettingsProviderProbe { + private(set) var callCount = 0 + + func record() -> MCPSettings { + callCount += 1 + return MCPSettings(defaultRowLimit: 42, maxRowLimit: 99) + } +} + @Suite("ExportDataTool") struct ExportDataToolTests { + @Test("Export resolves its row limit from the configured MCP settings") + func resolvesLimitsFromSettings() async throws { + let probe = SettingsProviderProbe() + let tool = ExportDataTool() + let context = await MCPProtocolHandlerTestSupport.makeContext(method: "tools/call") + let services = MCPToolServices( + connectionBridge: MCPConnectionBridge(), + authPolicy: MCPAuthPolicy(), + settingsProvider: { await probe.record() } + ) + + _ = try? await tool.call( + arguments: .object([ + "connection_id": .string(UUID().uuidString), + "format": .string("csv"), + "tables": .array([.string("users")]) + ]), + context: context, + services: services + ) + + #expect(await probe.callCount == 1) + } + @Test("Tool exposes expected metadata") func metadata() { #expect(ExportDataTool.name == "export_data") @@ -64,6 +97,64 @@ struct ExportDataToolTests { } } + @Test("An over-fetched extra row marks the export truncated and is trimmed off") + func overFetchedRowMarksTruncation() { + let fetched = (0..<501).map { JsonValue.int($0) } + let limited = ExportDataTool.applyRowLimit(to: fetched, maxRows: 500, driverReportedTruncation: false) + #expect(limited.rows.count == 500) + #expect(limited.isTruncated) + } + + @Test("A result that fits the limit is not marked truncated") + func resultWithinLimitIsNotTruncated() { + let fetched = (0..<300).map { JsonValue.int($0) } + let limited = ExportDataTool.applyRowLimit(to: fetched, maxRows: 500, driverReportedTruncation: false) + #expect(limited.rows.count == 300) + #expect(!limited.isTruncated) + } + + @Test("A result exactly at the limit is not marked truncated") + func resultExactlyAtLimitIsNotTruncated() { + let fetched = (0..<500).map { JsonValue.int($0) } + let limited = ExportDataTool.applyRowLimit(to: fetched, maxRows: 500, driverReportedTruncation: false) + #expect(limited.rows.count == 500) + #expect(!limited.isTruncated) + } + + @Test("Driver-reported truncation is preserved when the row count fits") + func driverTruncationIsPreserved() { + let fetched = (0..<500).map { JsonValue.int($0) } + let limited = ExportDataTool.applyRowLimit(to: fetched, maxRows: 500, driverReportedTruncation: true) + #expect(limited.rows.count == 500) + #expect(limited.isTruncated) + } + + @Test("LIMIT dialects append a trailing LIMIT clause") + func limitDialectUsesLimitClause() { + let sql = ExportDataTool.limitedSelectAll(from: "\"users\"", limit: 500, autoLimitStyle: .limit) + #expect(sql == "SELECT * FROM \"users\" LIMIT 500") + } + + @Test("TOP dialects put the limit before the column list") + func topDialectUsesTopClause() { + let sql = ExportDataTool.limitedSelectAll(from: "[dbo].[Users]", limit: 500, autoLimitStyle: .top) + #expect(sql == "SELECT TOP 500 * FROM [dbo].[Users]") + #expect(!sql.contains("LIMIT")) + } + + @Test("FETCH FIRST dialects use the ANSI row-limit clause") + func fetchFirstDialectUsesFetchFirst() { + let sql = ExportDataTool.limitedSelectAll(from: "\"SCOTT\".\"EMP\"", limit: 500, autoLimitStyle: .fetchFirst) + #expect(sql == "SELECT * FROM \"SCOTT\".\"EMP\" FETCH FIRST 500 ROWS ONLY") + #expect(!sql.contains("LIMIT")) + } + + @Test("Dialects without an auto-limit style emit no limit clause") + func noneDialectEmitsNoLimitClause() { + let sql = ExportDataTool.limitedSelectAll(from: "\"events\"", limit: 500, autoLimitStyle: .none) + #expect(sql == "SELECT * FROM \"events\"") + } + @Test("Neither query nor tables returns invalidParams") func missingQueryAndTables() async throws { let tool = ExportDataTool() diff --git a/TableProTests/Core/MCP/Protocol/Tools/MCPLimitResolverTests.swift b/TableProTests/Core/MCP/Protocol/Tools/MCPLimitResolverTests.swift new file mode 100644 index 000000000..df3897db7 --- /dev/null +++ b/TableProTests/Core/MCP/Protocol/Tools/MCPLimitResolverTests.swift @@ -0,0 +1,93 @@ +import Foundation +@testable import TablePro +import Testing + +@Suite("MCPLimitResolver") +struct MCPLimitResolverTests { + private func settings( + defaultRowLimit: Int = 500, + maxRowLimit: Int = 10_000, + queryTimeoutSeconds: Int = 30 + ) -> MCPSettings { + MCPSettings( + defaultRowLimit: defaultRowLimit, + maxRowLimit: maxRowLimit, + queryTimeoutSeconds: queryTimeoutSeconds + ) + } + + @Test("Omitted max_rows falls back to the configured default row limit") + func omittedRequestUsesDefault() { + let resolved = MCPLimitResolver.resolveMaxRows(requested: nil, settings: settings(defaultRowLimit: 750)) + #expect(resolved == 750) + } + + @Test("Requested max_rows above the configured maximum is clamped to it") + func requestAboveMaximumIsClamped() { + let resolved = MCPLimitResolver.resolveMaxRows(requested: 999_999, settings: settings(maxRowLimit: 10_000)) + #expect(resolved == 10_000) + } + + @Test("Requested max_rows within range is honoured") + func requestWithinRangeIsHonoured() { + let resolved = MCPLimitResolver.resolveMaxRows(requested: 9_000, settings: settings(maxRowLimit: 10_000)) + #expect(resolved == 9_000) + } + + @Test("Raising the configured maximum raises the effective ceiling") + func raisedMaximumRaisesCeiling() { + let resolved = MCPLimitResolver.resolveMaxRows(requested: 50_000, settings: settings(maxRowLimit: 100_000)) + #expect(resolved == 50_000) + } + + @Test("A default above the configured maximum is capped by the maximum") + func defaultAboveMaximumIsCapped() { + let resolved = MCPLimitResolver.resolveMaxRows( + requested: nil, + settings: settings(defaultRowLimit: 50_000, maxRowLimit: 1_000) + ) + #expect(resolved == 1_000) + } + + @Test("Zero and negative requests are raised to one instead of trapping") + func nonPositiveRequestIsRaised() { + #expect(MCPLimitResolver.resolveMaxRows(requested: 0, settings: settings()) == 1) + #expect(MCPLimitResolver.resolveMaxRows(requested: -5, settings: settings()) == 1) + } + + @Test("A zero maximum row limit resolves without trapping") + func zeroMaximumDoesNotTrap() { + let resolved = MCPLimitResolver.resolveMaxRows(requested: 100, settings: settings(maxRowLimit: 0)) + #expect(resolved == 1) + } + + @Test("A negative maximum row limit resolves without trapping") + func negativeMaximumDoesNotTrap() { + let resolved = MCPLimitResolver.resolveMaxRows(requested: nil, settings: settings(maxRowLimit: -20)) + #expect(resolved == 1) + } + + @Test("Omitted timeout falls back to the configured query timeout") + func omittedTimeoutUsesConfigured() { + let resolved = MCPLimitResolver.resolveTimeoutSeconds( + requested: nil, + settings: settings(queryTimeoutSeconds: 45) + ) + #expect(resolved == 45) + } + + @Test("Requested timeout is clamped to the protocol ceiling of 300 seconds") + func requestedTimeoutIsClamped() { + #expect(MCPLimitResolver.resolveTimeoutSeconds(requested: 5_000, settings: settings()) == 300) + #expect(MCPLimitResolver.resolveTimeoutSeconds(requested: 0, settings: settings()) == 1) + } + + @Test("A zero configured timeout resolves to the minimum instead of disabling the timeout") + func zeroConfiguredTimeoutIsRaised() { + let resolved = MCPLimitResolver.resolveTimeoutSeconds( + requested: nil, + settings: settings(queryTimeoutSeconds: 0) + ) + #expect(resolved == 1) + } +} diff --git a/docs/customization/settings.mdx b/docs/customization/settings.mdx index 23b58a2aa..381c4e3e0 100644 --- a/docs/customization/settings.mdx +++ b/docs/customization/settings.mdx @@ -150,8 +150,8 @@ The **Integrations** tab runs the MCP server. **Enable MCP Server** (default off | Setting | Default | Description | |---------|---------|-------------| | **Port** | 23508 | TCP port the server listens on | -| **Default row limit** | 500 | Rows returned when a tool call sets no limit | -| **Maximum row limit** | 10,000 | Hard cap on rows per tool call | +| **Default row limit** | 500 | Rows returned when a tool call sets no limit, including `export_data` | +| **Maximum row limit** | 10,000 | Hard cap on rows per tool call, including `export_data` | | **Query timeout** | 30 seconds | Per-query limit for MCP-issued queries | | **Log MCP queries in history** | On | Record MCP queries in [query history](/features/query-history) | diff --git a/docs/external-api/mcp-tools.mdx b/docs/external-api/mcp-tools.mdx index b2b73e3e2..875441f27 100644 --- a/docs/external-api/mcp-tools.mdx +++ b/docs/external-api/mcp-tools.mdx @@ -298,13 +298,13 @@ Export query or table data as CSV, JSON, or SQL. "connection_id": "...", "format": "csv", "tables": ["users", "orders"], - "max_rows": 50000 + "max_rows": 5000 } ``` -`format` is one of `csv`, `json`, `sql`. `max_rows` defaults to 50,000, max 100,000. Provide either `tables` or `query`. Table names accept letters, digits, underscore, and `.` for schema-qualified names. Pass `output_path` to write to disk instead of returning data inline; the path must resolve inside the user's `~/Downloads` directory. Anything else fails with JSON-RPC `-32602` over HTTP 200 and message `Invalid params: output_path must be inside the Downloads directory (/Users/you/Downloads)`. +`format` is one of `csv`, `json`, `sql`. Defaults for `max_rows` and the query timeout come from **Settings > Integrations > Server Configuration**, the same as `execute_query`: omitting `max_rows` uses the default row limit (500), and any value you pass is clamped to the maximum row limit (default 10,000). Raise those settings to export more rows. Provide either `tables` or `query`. Table names accept letters, digits, underscore, and `.` for schema-qualified names. Pass `output_path` to write to disk instead of returning data inline; the path must resolve inside the user's `~/Downloads` directory. Anything else fails with JSON-RPC `-32602` over HTTP 200 and message `Invalid params: output_path must be inside the Downloads directory (/Users/you/Downloads)`. -**Output**: when `output_path` is set, returns `{ "path": "...", "rows_exported": N }`. Otherwise returns the export inline. A single export returns `{ "label": "...", "format": "csv", "row_count": N, "data": "..." }`. Multiple exports (multi-table requests) return `{ "exports": [ { "label": "...", "format": "csv", "row_count": N, "data": "..." }, ... ] }`. +**Output**: when `output_path` is set, returns `{ "path": "...", "rows_exported": N, "is_truncated": false }`. Otherwise returns the export inline. A single export returns `{ "label": "...", "format": "csv", "row_count": N, "is_truncated": false, "data": "..." }`. Multiple exports (multi-table requests) return `{ "exports": [ { "label": "...", "format": "csv", "row_count": N, "is_truncated": false, "data": "..." }, ... ] }`. `is_truncated` is `true` when the row limit clipped the result. **Scope**: `readOnly`.