diff --git a/lib/core/database/mysql_connection.dart b/lib/core/database/mysql_connection.dart index c066a19f..7c8aaeb4 100644 --- a/lib/core/database/mysql_connection.dart +++ b/lib/core/database/mysql_connection.dart @@ -436,6 +436,20 @@ class MysqlConnection { await execute(sessionTransactionAccessModeSql(readOnly)); } + /// Session settings of a pool slot: the read-only default and, when the slot + /// has one, a server-side limit on SELECT execution time. + Future configureSession({ + required bool readOnly, + Duration? statementTimeout, + }) async { + await setSessionReadOnly(readOnly); + if (statementTimeout != null) { + await execute( + 'SET SESSION max_execution_time = ${statementTimeout.inMilliseconds}', + ); + } + } + /// `SET SESSION TRANSACTION READ ONLY` / `READ WRITE`. @visibleForTesting static String sessionTransactionAccessModeSql(bool readOnly) => readOnly diff --git a/lib/core/database/mysql_connection_pool.dart b/lib/core/database/mysql_connection_pool.dart index 85df2667..fd933c37 100644 --- a/lib/core/database/mysql_connection_pool.dart +++ b/lib/core/database/mysql_connection_pool.dart @@ -6,6 +6,9 @@ import 'package:querya_desktop/core/database/mysql_connection.dart'; import 'package:querya_desktop/core/storage/local_db.dart'; /// Session policy for pooled connections: browse vs SQL editor (writes). +/// Longest statement an MCP session may run; the server stops it after this. +const Duration mcpStatementTimeout = Duration(seconds: 15); + enum MysqlSessionMode { /// Tree catalog, stats, and Table Browser SELECT/COUNT. readOnly, @@ -15,10 +18,17 @@ enum MysqlSessionMode { /// Table Browser Save (own TCP session so START TRANSACTION / SET / USE do not leak). tableWrite, + /// MCP clients: a read-only session of their own, so an agent never shares + /// the user's session. Statements are bounded by [statementTimeout]. + mcp, } extension MysqlSessionModeReadOnly on MysqlSessionMode { - bool get isReadOnlySession => this == MysqlSessionMode.readOnly; + bool get isReadOnlySession => this == MysqlSessionMode.readOnly || this == MysqlSessionMode.mcp; + + /// Server-side limit for every statement on this session, or null for none. + Duration? get statementTimeout => + this == MysqlSessionMode.mcp ? mcpStatementTimeout : null; } typedef MysqlPoolConnectionFactory = Future Function( @@ -144,7 +154,10 @@ class MysqlConnectionPool { if (inFlight != null) return inFlight; final attempt = () async { await entry.connection.connect(); - await entry.connection.setSessionReadOnly(mode.isReadOnlySession); + await entry.connection.configureSession( + readOnly: mode.isReadOnlySession, + statementTimeout: mode.statementTimeout, + ); }(); entry.reconnecting = attempt; return attempt.whenComplete(() { diff --git a/lib/core/database/mysql_service.dart b/lib/core/database/mysql_service.dart index 416a160d..365e92a4 100644 --- a/lib/core/database/mysql_service.dart +++ b/lib/core/database/mysql_service.dart @@ -19,7 +19,10 @@ Future _defaultCreateAndConnect( database: database.isEmpty ? null : database, ); await conn.connect(); - await conn.setSessionReadOnly(mode.isReadOnlySession); + await conn.configureSession( + readOnly: mode.isReadOnlySession, + statementTimeout: mode.statementTimeout, + ); return conn; } diff --git a/lib/core/database/postgres_connection.dart b/lib/core/database/postgres_connection.dart index bc896a04..fcf4311f 100644 --- a/lib/core/database/postgres_connection.dart +++ b/lib/core/database/postgres_connection.dart @@ -389,6 +389,20 @@ class PostgresConnection { ); } + /// Session settings of a pool slot: the read-only default and, when the slot + /// has one, a server-side statement timeout. + Future configureSession({ + required bool readOnly, + Duration? statementTimeout, + }) async { + await setSessionReadOnly(readOnly); + if (statementTimeout != null) { + await execute( + 'SET statement_timeout = ${statementTimeout.inMilliseconds}', + ); + } + } + /// Tests connectivity and returns a result with an optional error message. Future<({bool ok, String? error})> testConnection() async { try { diff --git a/lib/core/database/postgres_connection_pool.dart b/lib/core/database/postgres_connection_pool.dart index 1602cdf9..cc5065db 100644 --- a/lib/core/database/postgres_connection_pool.dart +++ b/lib/core/database/postgres_connection_pool.dart @@ -6,6 +6,9 @@ import 'package:querya_desktop/core/database/postgres_connection.dart'; import 'package:querya_desktop/core/storage/local_db.dart'; /// Session policy for pooled connections: browse-only vs ad-hoc SQL (writes). +/// Longest statement an MCP session may run; the server stops it after this. +const Duration mcpStatementTimeout = Duration(seconds: 15); + enum PgSessionMode { /// `SET default_transaction_read_only = ON` after connect. /// Tree catalog, stats, and Table Browser SELECT. @@ -16,11 +19,18 @@ enum PgSessionMode { /// Table Browser Save / `REFRESH MATERIALIZED VIEW` (own TCP session). tableWrite, + /// MCP clients: a read-only session of their own, so an agent never shares + /// the user's session. Statements are bounded by [statementTimeout]. + mcp, } extension PgSessionModeReadOnly on PgSessionMode { /// Whether this slot should `SET default_transaction_read_only = ON`. - bool get isReadOnlySession => this == PgSessionMode.readOnly; + bool get isReadOnlySession => this == PgSessionMode.readOnly || this == PgSessionMode.mcp; + + /// Server-side limit for every statement on this session, or null for none. + Duration? get statementTimeout => + this == PgSessionMode.mcp ? mcpStatementTimeout : null; } /// Creates a connected [PostgresConnection] for the pool (real or fake in tests). @@ -133,7 +143,10 @@ class PostgresConnectionPool { if (inFlight != null) return inFlight; final attempt = () async { await entry.connection.connect(); - await entry.connection.setSessionReadOnly(mode.isReadOnlySession); + await entry.connection.configureSession( + readOnly: mode.isReadOnlySession, + statementTimeout: mode.statementTimeout, + ); }(); entry.reconnecting = attempt; return attempt.whenComplete(() { diff --git a/lib/core/database/postgres_service.dart b/lib/core/database/postgres_service.dart index e695f933..8de1d309 100644 --- a/lib/core/database/postgres_service.dart +++ b/lib/core/database/postgres_service.dart @@ -12,7 +12,10 @@ Future _defaultCreateAndConnect( }) async { final conn = PostgresConnection.fromConnectionRow(row, database: database); await conn.connect(); - await conn.setSessionReadOnly(mode.isReadOnlySession); + await conn.configureSession( + readOnly: mode.isReadOnlySession, + statementTimeout: mode.statementTimeout, + ); return conn; } diff --git a/lib/core/database/sqlite_connection_pool.dart b/lib/core/database/sqlite_connection_pool.dart index a99bc08e..eab1bacc 100644 --- a/lib/core/database/sqlite_connection_pool.dart +++ b/lib/core/database/sqlite_connection_pool.dart @@ -15,10 +15,13 @@ enum SqliteSessionMode { /// Table Browser Save (own `Database` so BEGIN/ATTACH do not leak). tableWrite, + /// MCP clients: a read-only session of their own, so an agent never shares + /// the user's session. Statements are bounded by [statementTimeout]. + mcp, } extension SqliteSessionModeReadOnly on SqliteSessionMode { - bool get isReadOnlySession => this == SqliteSessionMode.readOnly; + bool get isReadOnlySession => this == SqliteSessionMode.readOnly || this == SqliteSessionMode.mcp; } /// Factory to build a connected SQLite connection. diff --git a/lib/core/mcp/mcp_sql_delegates.dart b/lib/core/mcp/mcp_sql_delegates.dart index 4ca60b9b..7f01de22 100644 --- a/lib/core/mcp/mcp_sql_delegates.dart +++ b/lib/core/mcp/mcp_sql_delegates.dart @@ -5,8 +5,8 @@ import 'package:querya_desktop/features/postgresql/postgres_sql_workspace.dart'; import 'package:querya_desktop/features/sqlite/sqlite_sql_workspace.dart'; import 'package:querya_desktop/features/workspace/sql_execution_delegate.dart'; -/// Production [McpDelegateFactory]: the SQL editor's delegates, always on a -/// read-only session (Postgres `default_transaction_read_only`, MySQL +/// Production [McpDelegateFactory]: the SQL editor's delegates, always on the +/// MCP session (its own pool slot) and read-only (Postgres `default_transaction_read_only`, MySQL /// `SET SESSION TRANSACTION READ ONLY`, SQLite `SQLITE_OPEN_READONLY`). SqlExecutionDelegate createReadOnlyMcpDelegate( ConnectionRow row, @@ -18,13 +18,22 @@ SqlExecutionDelegate createReadOnlyMcpDelegate( return PostgresSqlExecutionDelegate( connectionRow: row, isReadOnly: true, + isMcp: true, effectiveDatabaseProvider: () => db == null || db.isEmpty ? 'postgres' : db, autocommitProvider: () => true, ); case SqlDialect.mysql: - return MysqlSqlExecutionDelegate(connectionRow: row, isReadOnly: true); + return MysqlSqlExecutionDelegate( + connectionRow: row, + isReadOnly: true, + isMcp: true, + ); case SqlDialect.sqlite: - return SqliteSqlExecutionDelegate(connectionRow: row, isReadOnly: true); + return SqliteSqlExecutionDelegate( + connectionRow: row, + isReadOnly: true, + isMcp: true, + ); } } diff --git a/lib/features/mysql/mysql_sql_workspace.dart b/lib/features/mysql/mysql_sql_workspace.dart index 87056210..737b818f 100644 --- a/lib/features/mysql/mysql_sql_workspace.dart +++ b/lib/features/mysql/mysql_sql_workspace.dart @@ -17,11 +17,19 @@ class MysqlSqlExecutionDelegate extends SqlExecutionDelegate { MysqlSqlExecutionDelegate({ required this.connectionRow, required this.isReadOnly, + this.isMcp = false, }); final ConnectionRow connectionRow; final bool isReadOnly; + /// MCP delegates run on the MCP session of their own, not the user's. + final bool isMcp; + + MysqlSessionMode get _sessionMode => isMcp + ? MysqlSessionMode.mcp + : (isReadOnly ? MysqlSessionMode.readOnly : MysqlSessionMode.readWrite); + MysqlLease? _lease; MysqlLease? get lease => _lease; @@ -35,7 +43,7 @@ class MysqlSqlExecutionDelegate extends SqlExecutionDelegate { final lease = await MysqlService.instance.acquire( connectionRow, database: poolDatabaseKey, - mode: isReadOnly ? MysqlSessionMode.readOnly : MysqlSessionMode.readWrite, + mode: _sessionMode, ); _lease = lease; } @@ -165,7 +173,7 @@ class MysqlSqlExecutionDelegate extends SqlExecutionDelegate { MysqlService.instance.interrupt( connectionRow, database: poolDatabaseKey, - mode: isReadOnly ? MysqlSessionMode.readOnly : MysqlSessionMode.readWrite, + mode: _sessionMode, ); await _lease?.connection.forceClose(); dropLease(); diff --git a/lib/features/postgresql/postgres_sql_workspace.dart b/lib/features/postgresql/postgres_sql_workspace.dart index ccb25ce6..bf29014d 100644 --- a/lib/features/postgresql/postgres_sql_workspace.dart +++ b/lib/features/postgresql/postgres_sql_workspace.dart @@ -26,10 +26,18 @@ class PostgresSqlExecutionDelegate extends SqlExecutionDelegate { required this.isReadOnly, required this.effectiveDatabaseProvider, required this.autocommitProvider, + this.isMcp = false, }); final ConnectionRow connectionRow; final bool isReadOnly; + + /// MCP delegates run on the MCP session of their own, not the user's. + final bool isMcp; + + PgSessionMode get _sessionMode => isMcp + ? PgSessionMode.mcp + : (isReadOnly ? PgSessionMode.readOnly : PgSessionMode.readWrite); final String Function() effectiveDatabaseProvider; final bool Function() autocommitProvider; @@ -45,7 +53,7 @@ class PostgresSqlExecutionDelegate extends SqlExecutionDelegate { final lease = await PostgresService.instance.acquire( connectionRow, database: db, - mode: isReadOnly ? PgSessionMode.readOnly : PgSessionMode.readWrite, + mode: _sessionMode, ); _lease = lease; _interruptDatabase = db; @@ -164,7 +172,7 @@ class PostgresSqlExecutionDelegate extends SqlExecutionDelegate { PostgresService.instance.interrupt( connectionRow, database: _interruptDatabase ?? effectiveDatabaseProvider(), - mode: isReadOnly ? PgSessionMode.readOnly : PgSessionMode.readWrite, + mode: _sessionMode, ); await _lease?.connection.forceClose(); dropLease(); diff --git a/lib/features/sqlite/sqlite_sql_workspace.dart b/lib/features/sqlite/sqlite_sql_workspace.dart index a3c596af..d0d226ce 100644 --- a/lib/features/sqlite/sqlite_sql_workspace.dart +++ b/lib/features/sqlite/sqlite_sql_workspace.dart @@ -17,11 +17,19 @@ class SqliteSqlExecutionDelegate extends SqlExecutionDelegate { SqliteSqlExecutionDelegate({ required this.connectionRow, required this.isReadOnly, + this.isMcp = false, }); final ConnectionRow connectionRow; final bool isReadOnly; + /// MCP delegates run on the MCP session of their own, not the user's. + final bool isMcp; + + SqliteSessionMode get _sessionMode => isMcp + ? SqliteSessionMode.mcp + : (isReadOnly ? SqliteSessionMode.readOnly : SqliteSessionMode.readWrite); + SqliteLease? _lease; SqliteLease? get lease => _lease; @@ -32,9 +40,7 @@ class SqliteSqlExecutionDelegate extends SqlExecutionDelegate { _lease = null; final lease = await SqliteService.instance.acquire( connectionRow, - mode: isReadOnly - ? SqliteSessionMode.readOnly - : SqliteSessionMode.readWrite, + mode: _sessionMode, ); _lease = lease; } diff --git a/test/core/database/mysql_connection_pool_test.dart b/test/core/database/mysql_connection_pool_test.dart index 3d5e82d9..9d6f04ab 100644 --- a/test/core/database/mysql_connection_pool_test.dart +++ b/test/core/database/mysql_connection_pool_test.dart @@ -266,4 +266,35 @@ void main() { } }); }); + + group('MCP session (#1217)', () { + test('MCP has its own slot, read-only, with a server-side statement timeout', + () async { + expect(MysqlSessionMode.mcp.isReadOnlySession, isTrue); + expect(MysqlSessionMode.readOnly.statementTimeout, isNull); + expect(MysqlSessionMode.mcp.statementTimeout, mcpStatementTimeout); + }); + + test('an MCP lease never shares the connection of the read-only slot', + () async { + final created = []; + final pool = MysqlConnectionPool( + createAndConnect: (row, {required database, required mode}) async { + final c = FakeMysqlConnection(); + await c.connect(); + created.add(c); + return c; + }, + ); + final r = _row(); + final ui = await pool.acquire(r, + database: 'app', mode: MysqlSessionMode.readOnly); + final mcp = await pool.acquire(r, + database: 'app', mode: MysqlSessionMode.mcp); + expect(identical(ui.connection, mcp.connection), isFalse); + expect(created.length, 2); + ui.release(); + mcp.release(); + }); + }); } diff --git a/test/core/database/postgres_connection_pool_test.dart b/test/core/database/postgres_connection_pool_test.dart index 483b2d92..aeef623c 100644 --- a/test/core/database/postgres_connection_pool_test.dart +++ b/test/core/database/postgres_connection_pool_test.dart @@ -627,4 +627,41 @@ void main() { } }); }); + + group('MCP session (#1217)', () { + test('MCP has its own slot, read-only, with a server-side statement timeout', + () async { + expect(PgSessionMode.mcp.isReadOnlySession, isTrue); + expect(PgSessionMode.readOnly.statementTimeout, isNull); + expect(PgSessionMode.mcp.statementTimeout, mcpStatementTimeout); + final pool = PostgresConnectionPool( + createAndConnect: (row, {required database, required mode}) async => + FakePostgresConnection(), + ); + expect(pool.keyFor(1, 'app', PgSessionMode.mcp), + isNot(pool.keyFor(1, 'app', PgSessionMode.readOnly))); + }); + + test('an MCP lease never shares the connection of the read-only slot', + () async { + final created = []; + final pool = PostgresConnectionPool( + createAndConnect: (row, {required database, required mode}) async { + final c = FakePostgresConnection(); + await c.connect(); + created.add(c); + return c; + }, + ); + final r = _row(); + final ui = await pool.acquire(r, + database: 'app', mode: PgSessionMode.readOnly); + final mcp = await pool.acquire(r, + database: 'app', mode: PgSessionMode.mcp); + expect(identical(ui.connection, mcp.connection), isFalse); + expect(created.length, 2); + ui.release(); + mcp.release(); + }); + }); }