diff --git a/CHANGELOG.md b/CHANGELOG.md index 9883a0d..da2bbfa 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,21 @@ +## 4.6.0 + +### Feature: DB-backed MCP session store (cross-instance / load-balanced support) + +MCP protocol sessions were stored via the SDK's `FileSessionStore` under `sys_get_temp_dir()` — **local to a single host**. On a deployment behind a load balancer (e.g. ETOM's two instances), the `initialize` request lands on instance A and the follow-up request hits instance B, which has no session file → the SDK returns `-32600 "Session not found or has expired"` → the MCP client falls back to OAuth discovery (which Kyte doesn't serve) → the connection fails. Single-instance installs were unaffected. + +This release adds `Kyte\Mcp\Session\DbSessionStore`, which persists protocol sessions in a new `KyteMCPSession` table so **any instance sharing the database resolves any session**, and makes it the default backend. + +1. **New table: `KyteMCPSession`** (`migrations/4.6.0_mcp_session_store.sql`). Stores `session_id` (RFC4122 UUID, UNIQUE), the `payload` (the SDK's `json_encode`d session array), `last_activity`, and `kyte_account`. `CREATE TABLE IF NOT EXISTS`, safe to re-run. Auto-registered as a model via the `Mvc/Model` loader. + +2. **DB store is now the default.** `Endpoint::buildSessionStore()` selects the backend. The DB store is correct for any topology (single host, LB, future SaaS). Single-instance installs that prefer not to add the table can opt back to the file store with `define('KYTE_MCP_SESSION_STORE', 'file');`. + +3. **TTL semantics preserved.** `last_activity` is the last-write time; the SDK calls `session->save()` at the end of every handled request, so an active session's timestamp slides forward (idle timeout). `exists()` reports expiry without deleting; `read()` purges on expiry — both mirror `FileSessionStore` exactly. Idle TTL is `KYTE_MCP_SESSION_TTL` (default 3600s) for either backend. + +4. **Tenancy & cleanup.** Reads/writes/destroys are scoped to the bearer token's `kyte_account` (a session resolves only under the account that created it). `gc()` (the SDK runs it on ~1% of requests) is a global sweep of TTL-expired rows, capped at 1000 per call; rows are hard-deleted (purged), not tombstoned, so the `session_id` UNIQUE index stays clean. + +**Operational note:** run `migrations/4.6.0_mcp_session_store.sql` as part of the upgrade. With the default DB backend active, the table must exist before MCP traffic is served. Multi-instance installs (ETOM) require no per-instance config beyond the shared DB; this unblocks the `kyte-etometry` MCP connection (Tempo KYTE-183). No change to MCP auth (`KyteMCPToken`), which was already DB-backed. + ## 4.5.3 ### Bug Fix (critical): `/jwt/refresh` 401s for apps with a custom `user_model` diff --git a/migrations/4.6.0_mcp_session_store.sql b/migrations/4.6.0_mcp_session_store.sql new file mode 100644 index 0000000..cf42bc3 --- /dev/null +++ b/migrations/4.6.0_mcp_session_store.sql @@ -0,0 +1,55 @@ +-- ========================================================================= +-- Kyte v4.6.0 - MCP protocol session store (cross-instance / load-balanced) +-- ========================================================================= +-- IMPORTANT: Backup your database before running this migration. +-- +-- Creates the KyteMCPSession table consumed by Kyte\Mcp\Session\DbSessionStore, +-- the default MCP protocol-session backend as of 4.6.0. It replaces the SDK's +-- per-host FileSessionStore so that deployments behind a load balancer (e.g. +-- multiple PHP instances sharing one database) can resolve the same MCP +-- `MCP-Session-Id` on any instance. +-- +-- Background: the SDK FileSessionStore writes one file per session under +-- sys_get_temp_dir(), local to a single host. When `initialize` lands on +-- instance A and the follow-up request hits instance B, B has no such file and +-- the SDK returns -32600 "Session not found or has expired", collapsing the +-- MCP connection. This table makes session state shared. See Tempo KYTE-183 +-- and docs/design/kyte-mcp-and-auth-migration.md section 11. +-- +-- This is MCP *protocol* session state (the negotiated initialize/capabilities +-- handshake), NOT auth. MCP auth (KyteMCPToken) was already DB-backed and +-- cross-instance. Rows here are short-lived and hard-deleted on destroy / TTL +-- garbage collection. +-- +-- Single-instance installs that prefer no extra table can stay on the file +-- store by setting `define('KYTE_MCP_SESSION_STORE', 'file');` in config; in +-- that case this table is simply unused. Idle TTL is KYTE_MCP_SESSION_TTL +-- (default 3600s). +-- +-- See src/Mvc/Model/KyteMCPSession.php for the model spec. +-- Existing installs that already created an equivalent table are unaffected +-- (IF NOT EXISTS). +-- ========================================================================= + +CREATE TABLE IF NOT EXISTS `KyteMCPSession` ( + `id` BIGINT UNSIGNED AUTO_INCREMENT PRIMARY KEY, + + `session_id` VARCHAR(36) NOT NULL COMMENT 'RFC4122 UUID issued by the MCP SDK session factory', + `payload` TEXT NOT NULL COMMENT 'json_encode of the SDK session data array (opaque to Kyte)', + `last_activity` BIGINT UNSIGNED NOT NULL DEFAULT 0 COMMENT 'Unix epoch of last write; TTL is measured against this (idle timeout)', + + `kyte_account` BIGINT UNSIGNED NOT NULL, + + `created_by` BIGINT UNSIGNED DEFAULT NULL, + `date_created` BIGINT UNSIGNED DEFAULT NULL, + `modified_by` BIGINT UNSIGNED DEFAULT NULL, + `date_modified` BIGINT UNSIGNED DEFAULT NULL, + `deleted_by` BIGINT UNSIGNED DEFAULT NULL, + `date_deleted` BIGINT UNSIGNED DEFAULT NULL, + `deleted` TINYINT UNSIGNED NOT NULL DEFAULT 0, + + UNIQUE KEY `idx_session_id` (`session_id`), + KEY `idx_account` (`kyte_account`), + KEY `idx_account_session` (`kyte_account`, `session_id`), + KEY `idx_last_activity` (`last_activity`) +) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4; diff --git a/phpunit.xml.dist b/phpunit.xml.dist index a6085fd..eefbf7b 100644 --- a/phpunit.xml.dist +++ b/phpunit.xml.dist @@ -7,6 +7,7 @@ tests/Bz2CodecTest.php tests/ClientIpTest.php tests/DatabaseTest.php + tests/DbSessionStoreTest.php tests/ErrorHandlerSensitivityTest.php tests/FunctionTest.php tests/HmacSessionStrategyTest.php diff --git a/src/Mcp/Endpoint.php b/src/Mcp/Endpoint.php index 1474a37..3eb6c96 100644 --- a/src/Mcp/Endpoint.php +++ b/src/Mcp/Endpoint.php @@ -5,6 +5,7 @@ use Kyte\Core\Auth\AuthDispatcher; use Kyte\Core\Auth\McpTokenStrategy; use Kyte\Exception\SessionException; +use Kyte\Mcp\Session\DbSessionStore; use Kyte\Mcp\Session\SaveSafeSessionFactory; use Laminas\HttpHandlerRunner\Emitter\SapiEmitter; use Mcp\Capability\Registry as McpRegistry; @@ -13,6 +14,7 @@ use Mcp\Server; use Mcp\Server\Handler\Request\CallToolHandler; use Mcp\Server\Session\FileSessionStore; +use Mcp\Server\Session\SessionStoreInterface; use Mcp\Server\Transport\StreamableHttpTransport; use Nyholm\Psr7\Factory\Psr17Factory; use Nyholm\Psr7Server\ServerRequestCreator; @@ -31,11 +33,13 @@ * envelopes, neither of which apply to JSON-RPC over MCP. Calling * Api::route() detects `/mcp` early and delegates here before any of that runs. * - * Session storage uses the SDK's bundled FileSessionStore, scoped per-install - * under sys_get_temp_dir(). MCP sessions are short-lived and per-Claude- - * conversation, so file storage is sufficient at per-tenant scale. A MySQL- - * backed store can replace this if/when SaaS-scale deployment forces the - * issue (see design doc section 11). + * Session storage defaults to the DB-backed DbSessionStore (the KyteMCPSession + * table), so multi-instance / load-balanced deployments resolve the same + * protocol session on any host. The SDK's bundled FileSessionStore (per-host, + * under sys_get_temp_dir()) remains available as an escape hatch via the + * KYTE_MCP_SESSION_STORE='file' constant for single-instance installs that + * prefer to avoid the table. KYTE_MCP_SESSION_TTL overrides the idle TTL + * (default 3600s). See buildSessionStore() and design doc section 11. * * The handle()/process() split is for testability: process() is pure * request-in / response-out and can be exercised from PHPUnit, while handle() @@ -85,7 +89,7 @@ public static function process(Api $api, ServerRequestInterface $request): Respo $container = new McpContainer(); $container->set(Api::class, $api); - $sessionDir = self::sessionDirectory(); + $sessionStore = self::buildSessionStore($api); // Build registry + inner CallToolHandler ourselves so we can wrap the // dispatch with ScopedCallToolHandler. The Builder otherwise creates @@ -114,7 +118,7 @@ public static function process(Api $api, ServerRequestInterface $request): Respo ->setContainer($container) ->setRegistry($registry) ->addRequestHandler($scopedCallTool) - ->setSession(new FileSessionStore($sessionDir), new SaveSafeSessionFactory()) + ->setSession($sessionStore, new SaveSafeSessionFactory()) ->setDiscovery(__DIR__ . '/Tools') ->build(); @@ -199,6 +203,36 @@ private static function checkOrigin(ServerRequestInterface $request): ?string return "Origin '{$origin}' is not in the MCP_ALLOWED_ORIGINS allowlist."; } + /** + * Select the MCP protocol-session store. + * + * Defaults to the DB-backed store so load-balanced / multi-instance + * installs work out of the box (the SDK's FileSessionStore is per-host and + * breaks the moment `initialize` and its follow-ups land on different + * boxes — see DbSessionStore). Single-instance installs may opt back to the + * file store with KYTE_MCP_SESSION_STORE='file'. KYTE_MCP_SESSION_TTL (s) + * overrides the idle TTL for either backend. + * + * Requires the 4.6.0 migration (creates KyteMCPSession) to have run when + * the DB backend is active. + */ + private static function buildSessionStore(Api $api): SessionStoreInterface + { + $ttl = (defined('KYTE_MCP_SESSION_TTL') && (int)KYTE_MCP_SESSION_TTL > 0) + ? (int)KYTE_MCP_SESSION_TTL + : DbSessionStore::DEFAULT_TTL; + + $backend = defined('KYTE_MCP_SESSION_STORE') + ? strtolower(trim((string)KYTE_MCP_SESSION_STORE)) + : 'db'; + + if ($backend === 'file') { + return new FileSessionStore(self::sessionDirectory(), $ttl); + } + + return new DbSessionStore((int)$api->account->id, $ttl); + } + private static function sessionDirectory(): string { $dir = sys_get_temp_dir() . '/kyte-mcp-sessions'; diff --git a/src/Mcp/Session/DbSessionStore.php b/src/Mcp/Session/DbSessionStore.php new file mode 100644 index 0000000..d432a05 --- /dev/null +++ b/src/Mcp/Session/DbSessionStore.php @@ -0,0 +1,211 @@ +save() + * at the end of every handled request (Protocol::handleMessage), so an + * active session's timestamp slides forward — this is an idle timeout. + * - exists() reports expiry but does NOT delete (matches the file store). + * - read() deletes the row on expiry (matches the file store unlink-on-read). + * + * Tenancy: + * - read/write/exists/destroy are scoped to the account that owns the /mcp + * request (resolved from the bearer token before this store is built), so + * a session can only ever be resolved under the account that created it. + * - gc() is intentionally NOT account-scoped: it only ever removes rows that + * are already past their TTL (ephemeral, payload never read), and a global + * sweep keeps the table tidy on a shared/SaaS database no matter which + * tenant's request happens to trigger the ~1%-probability collection. + * + * Rows are hard-deleted (purge), not soft-deleted: these are throwaway protocol + * sessions, and leaving `deleted=1` tombstones would defeat the UNIQUE index on + * `session_id` when a UUID is (astronomically rarely) reused. + */ +final class DbSessionStore implements SessionStoreInterface +{ + /** Default idle TTL in seconds; matches the SDK FileSessionStore default. */ + public const DEFAULT_TTL = 3600; + + /** Safety cap on rows purged per gc() sweep so a single request never stalls. */ + private const GC_BATCH_LIMIT = 1000; + + public function __construct( + private int $kyteAccount, + private int $ttl = self::DEFAULT_TTL, + ) { + if ($this->ttl <= 0) { + $this->ttl = self::DEFAULT_TTL; + } + } + + public function exists(Uuid $id): bool + { + $row = $this->find($id); + if ($row === null) { + return false; + } + + return !$this->isExpired($row); + } + + public function read(Uuid $id): string|false + { + $row = $this->find($id); + if ($row === null) { + return false; + } + + if ($this->isExpired($row)) { + // Mirror FileSessionStore: expired-on-read is purged, then miss. + try { + $row->purge(); + } catch (\Throwable $e) { + error_log('DbSessionStore::read - failed to purge expired session - ' . $e->getMessage()); + } + return false; + } + + return (string)$row->payload; + } + + public function write(Uuid $id, string $data): bool + { + $now = time(); + + try { + $row = $this->find($id); + if ($row !== null) { + return $row->save([ + 'payload' => $data, + 'last_activity' => $now, + ]); + } + + $row = new ModelObject(KyteMCPSession); + return $row->create([ + 'session_id' => $id->toRfc4122(), + 'payload' => $data, + 'last_activity' => $now, + 'kyte_account' => $this->kyteAccount, + ]); + } catch (\Throwable $e) { + // A concurrent create on the same UUID (UNIQUE violation) is the + // only realistic failure here; re-resolve and update so the write + // still lands. Anything else is logged and reported as a miss. + try { + $row = $this->find($id); + if ($row !== null) { + return $row->save([ + 'payload' => $data, + 'last_activity' => $now, + ]); + } + } catch (\Throwable $inner) { + $e = $inner; + } + error_log('DbSessionStore::write - ' . $e->getMessage()); + return false; + } + } + + public function destroy(Uuid $id): bool + { + try { + $row = $this->find($id); + if ($row !== null) { + $row->purge(); + } + return true; + } catch (\Throwable $e) { + error_log('DbSessionStore::destroy - ' . $e->getMessage()); + return false; + } + } + + /** + * Remove sessions whose last activity is older than the TTL. Global (not + * account-scoped) by design — see class docblock. Returns the purged ids. + * + * @return Uuid[] + */ + public function gc(): array + { + $cutoff = time() - $this->ttl; + $deleted = []; + + try { + $model = new Model(KyteMCPSession); + $model->retrieve( + null, + null, + false, + [['field' => 'last_activity', 'operator' => '<', 'value' => $cutoff]], + false, + null, + self::GC_BATCH_LIMIT + ); + + foreach ($model->objects as $row) { + $sid = (string)$row->session_id; + try { + $row->purge(); + } catch (\Throwable $e) { + error_log('DbSessionStore::gc - failed to purge ' . $sid . ' - ' . $e->getMessage()); + continue; + } + try { + $deleted[] = Uuid::fromString($sid); + } catch (\Throwable) { + // non-UUID session_id should never happen; skip silently + } + } + } catch (\Throwable $e) { + error_log('DbSessionStore::gc - ' . $e->getMessage()); + } + + return $deleted; + } + + /** + * Resolve the session row for this account, or null on miss. Scoping the + * lookup to kyte_account keeps sessions isolated per tenant even though + * session_id is globally unique. + */ + private function find(Uuid $id): ?ModelObject + { + $row = new ModelObject(KyteMCPSession); + $found = $row->retrieve( + 'session_id', + $id->toRfc4122(), + [['field' => 'kyte_account', 'value' => $this->kyteAccount]] + ); + + return $found ? $row : null; + } + + private function isExpired(ModelObject $row): bool + { + return (time() - (int)$row->last_activity) > $this->ttl; + } +} diff --git a/src/Mvc/Model/KyteMCPSession.php b/src/Mvc/Model/KyteMCPSession.php new file mode 100644 index 0000000..8bed9be --- /dev/null +++ b/src/Mvc/Model/KyteMCPSession.php @@ -0,0 +1,128 @@ + 'KyteMCPSession', + 'struct' => [ + // RFC4122 UUID (e.g. "9f1c...-...") issued by the SDK SessionFactory. + // UNIQUE — the lookup key for every read/write/destroy. 36 chars covers + // the canonical hyphenated form. + 'session_id' => [ + 'type' => 's', + 'required' => true, + 'size' => 36, + 'date' => false, + ], + + // Encoded session state: json_encode of the SDK session data array + // (initialized, client_info, client_capabilities, protocol_version, + // log_level). Opaque to Kyte — written and read verbatim by the store. + 'payload' => [ + 'type' => 't', + 'required' => true, + 'date' => false, + ], + + // Unix epoch of the last write. The store's TTL is measured against + // this (mirrors FileSessionStore's per-write mtime touch): a session + // stays alive as long as a request touches it within KYTE_MCP_SESSION_TTL. + // The SDK calls session->save() at the end of every handled request, + // so this slides on activity and acts as an idle timeout. + 'last_activity' => [ + 'type' => 'i', + 'required' => false, + 'size' => 11, + 'unsigned' => true, + 'default' => 0, + 'date' => true, + ], + + // framework attributes + + 'kyte_account' => [ + 'type' => 'i', + 'required' => true, + 'size' => 11, + 'unsigned' => true, + 'date' => false, + ], + + // audit attributes + + 'created_by' => [ + 'type' => 'i', + 'required' => false, + 'date' => false, + ], + + 'date_created' => [ + 'type' => 'i', + 'required' => false, + 'date' => true, + ], + + 'modified_by' => [ + 'type' => 'i', + 'required' => false, + 'date' => false, + ], + + 'date_modified' => [ + 'type' => 'i', + 'required' => false, + 'date' => true, + ], + + 'deleted_by' => [ + 'type' => 'i', + 'required' => false, + 'date' => false, + ], + + 'date_deleted' => [ + 'type' => 'i', + 'required' => false, + 'date' => true, + ], + + 'deleted' => [ + 'type' => 'i', + 'required' => false, + 'size' => 1, + 'unsigned' => true, + 'default' => 0, + 'date' => false, + ], + ], +]; diff --git a/tests/DbSessionStoreTest.php b/tests/DbSessionStoreTest.php new file mode 100644 index 0000000..f22ed41 --- /dev/null +++ b/tests/DbSessionStoreTest.php @@ -0,0 +1,142 @@ +assertTrue(DBI::createTable(KyteMCPSession)); + } + + public function testWriteThenReadRoundTrips(): void + { + $store = new DbSessionStore(self::ACCOUNT_A); + $id = new UuidV4(); + $payload = json_encode(['initialized' => true, 'protocol_version' => '2025-06-18']); + + $this->assertTrue($store->write($id, $payload)); + $this->assertTrue($store->exists($id)); + $this->assertSame($payload, $store->read($id)); + } + + public function testMissReturnsFalse(): void + { + $store = new DbSessionStore(self::ACCOUNT_A); + $id = new UuidV4(); + + $this->assertFalse($store->exists($id)); + $this->assertFalse($store->read($id)); + } + + public function testWriteIsUpsertNotDuplicate(): void + { + $store = new DbSessionStore(self::ACCOUNT_A); + $id = new UuidV4(); + + $this->assertTrue($store->write($id, json_encode(['v' => 1]))); + $this->assertTrue($store->write($id, json_encode(['v' => 2]))); + + // Latest payload wins... + $this->assertSame(json_encode(['v' => 2]), $store->read($id)); + + // ...and there is exactly one row for that session_id. + $rows = DBI::query("SELECT COUNT(*) AS c FROM `KyteMCPSession` WHERE `session_id` = '" . $id->toRfc4122() . "'"); + $this->assertSame(1, (int)$rows[0]['c']); + } + + public function testDestroyRemovesSession(): void + { + $store = new DbSessionStore(self::ACCOUNT_A); + $id = new UuidV4(); + $store->write($id, json_encode(['initialized' => true])); + + $this->assertTrue($store->destroy($id)); + $this->assertFalse($store->exists($id)); + $this->assertFalse($store->read($id)); + } + + public function testSessionsAreAccountScoped(): void + { + $storeA = new DbSessionStore(self::ACCOUNT_A); + $storeB = new DbSessionStore(self::ACCOUNT_B); + $id = new UuidV4(); + + $storeA->write($id, json_encode(['owner' => 'A'])); + + // Same UUID, different account → invisible. + $this->assertTrue($storeA->exists($id)); + $this->assertFalse($storeB->exists($id)); + $this->assertFalse($storeB->read($id)); + } + + public function testExpiredSessionIsAMiss(): void + { + $store = new DbSessionStore(self::ACCOUNT_A, 3600); + $id = new UuidV4(); + $store->write($id, json_encode(['initialized' => true])); + + // Backdate last activity beyond the TTL. + $this->backdate($id, 7200); + + $this->assertFalse($store->exists($id)); + // read() reports the miss and purges the stale row. + $this->assertFalse($store->read($id)); + $rows = DBI::query("SELECT COUNT(*) AS c FROM `KyteMCPSession` WHERE `session_id` = '" . $id->toRfc4122() . "'"); + $this->assertSame(0, (int)$rows[0]['c']); + } + + public function testGcPurgesExpiredAndReturnsIds(): void + { + $store = new DbSessionStore(self::ACCOUNT_A, 3600); + + $fresh = new UuidV4(); + $stale = new UuidV4(); + $store->write($fresh, json_encode(['k' => 'fresh'])); + $store->write($stale, json_encode(['k' => 'stale'])); + $this->backdate($stale, 7200); + + $deleted = $store->gc(); + + $deletedStrings = array_map(static fn ($u) => $u->toRfc4122(), $deleted); + $this->assertContains($stale->toRfc4122(), $deletedStrings); + $this->assertNotContains($fresh->toRfc4122(), $deletedStrings); + + // Fresh session survives; stale is gone. + $this->assertTrue($store->exists($fresh)); + $this->assertFalse($store->exists($stale)); + } + + /** + * Push a session's last_activity into the past by $seconds via a direct + * UPDATE — simulating an idle session without sleeping the test. + */ + private function backdate(UuidV4 $id, int $seconds): void + { + $past = time() - $seconds; + DBI::query("UPDATE `KyteMCPSession` SET `last_activity` = $past WHERE `session_id` = '" . $id->toRfc4122() . "'"); + } +}