Stop leaking connection slots and reclaim unused sessions (v1.6.3)

CLAUDE.md

@@ -50,6 +50,15 @@ Copy `.env.example` to `.env` for local development.
 - **Selfhosted** (default): Single API token from env, one client per process. Supports multi-base via `SEATABLE_BASES`.
 - **Managed** (`SEATABLE_MODE=managed`): HTTP-only, each client authenticates with their own Bearer token **on every request** — the `mcp-session-id` header is a routing value, never a credential, and a request must resolve to the identity that created the session. Token validated against SeaTable (`src/auth/tokenValidator.ts`) with positive (1 min) / negative (1 min) cache. Rate limiting via `src/ratelimit/` (per-token, per-IP, global, concurrent connections).
 
+### Connection slots and session lifetime
+
+The concurrent-connection limit (20, per API token, `src/ratelimit/index.ts`) is acquired on session creation and released on `DELETE`, transport close, or by the idle sweeper. Two rules keep the pool from silting up, both covered by `tests/connectionSlots.spec.ts`:
+
+- An initialize request that never produces a session (malformed body, aborted client) releases its slot from `res.on('close')`. Without that the slot is unreachable — the transport never opened, so `onclose` never fires, and the sweeper never sees a session that was never registered.
+- A session that initialized but never made a call is reclaimed after **30 s** (`unusedSessionTimeoutMs`) instead of the ordinary 10-minute idle timeout. Reconnect-happy clients leave these behind by the dozen; each one holds a slot.
+
+Client IP for rate limiting comes from the **rightmost** `X-Forwarded-For` entry — the hop our own proxy appended. The leftmost entry is client-supplied and would let a caller pick its own rate-limit bucket. `docker-compose.yml` additionally has Caddy overwrite the header rather than append to it.
+
 ### OAuth (managed mode)
 
 `src/auth/oauthProvider.ts` bridges SeaTable API tokens into an OAuth 2.0 authorization code flow. Client registrations and issued tokens are **stateless sealed envelopes** (`src/auth/tokenCipher.ts`, AES-256-GCM keyed from `SEATABLE_TOKEN_SECRET`), so no server-side store is needed and they survive restarts. The `client_id` carries the client's registered `redirect_uris`; `/authorize` rejects anything it cannot open. PKCE `S256` is mandatory and every code is bound to client + exact callback + challenge. The raw SeaTable API token is never returned — `resolveAccessToken()` unseals it server-side.

docker-compose.yml

@@ -23,6 +23,11 @@ services:
     labels:
       caddy: ${SEATABLE_MCP_HOSTNAME}
       caddy.reverse_proxy: "{{upstreams 3000}}"
+      # Replace X-Forwarded-For instead of appending to it. Caddy appends by
+      # default, which leaves a client-supplied value in front of the real peer
+      # — and that value is what rate limiting would be keyed on. Overwriting
+      # here means the header carries exactly one hop: the actual client.
+      caddy.reverse_proxy.header_up: "X-Forwarded-For {remote_host}"
     environment:
       - SEATABLE_SERVER_URL
       - SEATABLE_API_TOKEN

package.json

@@ -1,6 +1,6 @@
 {
   "name": "@seatable/mcp-seatable",
-  "version": "1.6.2",
+  "version": "1.6.3",
   "type": "module",
   "license": "MIT",
   "mcpName": "io.github.seatable/seatable",

src/http/httpServer.ts

@@ -17,6 +17,12 @@ export interface StartHttpServerOptions {
     port?: number
     /** Session idle timeout in ms (default: 10 minutes) */
     sessionIdleTimeoutMs?: number
+    /**
+     * Idle timeout for a session that initialized but never made a call
+     * (default: 30 seconds). These are the sessions a reconnect-happy client
+     * leaves behind, and each one holds a connection slot while it lives.
+     */
+    unusedSessionTimeoutMs?: number
     /** Interval for checking idle sessions in ms (default: 60 seconds) */
     sessionCheckIntervalMs?: number
 }
@@ -26,6 +32,8 @@ type ActiveSession = {
     apiToken?: string
     /** Digest of the SeaTable API token that created this session; every later request must resolve to the same one. */
     apiTokenDigest?: Buffer
+    /** False until the client makes its first call. Sessions that never do are reclaimed far sooner. */
+    used: boolean
     lastActivity: number
     close: () => Promise<void>
 }
@@ -39,6 +47,11 @@ function sessionFingerprint(sessionId: string): string {
     return createHash('sha256').update(sessionId).digest('hex').slice(0, 12)
 }
 
+/** Same rule for API tokens: enough to correlate one tenant's lines, never the credential. */
+function tokenFingerprint(apiToken: string): string {
+    return createHash('sha256').update(apiToken).digest('hex').slice(0, 12)
+}
+
 const MAX_BODY_SIZE = 10 * 1024 * 1024 // 10 MB
 
 async function parseJsonBody(req: IncomingMessage): Promise<unknown> {
@@ -155,10 +168,21 @@ export async function startHttpServer(options: StartHttpServerOptions = {}) {
 
     const trustProxy = env.TRUST_PROXY ?? true
 
+    /**
+     * The rightmost X-Forwarded-For entry is the one our own reverse proxy
+     * appended, so it is the only one a client cannot forge. Reading the
+     * leftmost entry instead let a client choose its own rate-limit bucket:
+     * evade the per-IP limit by rotating the header, or poison the bucket
+     * another tenant is being counted in.
+     */
     function getClientIp(req: IncomingMessage): string {
         if (trustProxy) {
             const forwarded = req.headers['x-forwarded-for']
-            if (typeof forwarded === 'string') return forwarded.split(',')[0].trim()
+            const chain = Array.isArray(forwarded) ? forwarded.join(',') : forwarded
+            if (typeof chain === 'string') {
+                const hops = chain.split(',').map((hop) => hop.trim()).filter(Boolean)
+                if (hops.length > 0) return hops[hops.length - 1]
+            }
         }
         return req.socket.remoteAddress ?? 'unknown'
     }
@@ -246,23 +270,34 @@ export async function startHttpServer(options: StartHttpServerOptions = {}) {
             // Connection limit (managed mode)
             if (rateLimiter && apiToken) {
                 if (!rateLimiter.connections.acquire(apiToken)) {
-                    logger.warn({ ip: getClientIp(req) }, 'Connection limit exceeded')
+                    logger.warn(
+                        {
+                            ip: getClientIp(req),
+                            token: tokenFingerprint(apiToken),
+                            active: rateLimiter.connections.active(apiToken),
+                            limit: rateLimiter.connections.maxConnections,
+                        },
+                        'Connection limit exceeded'
+                    )
                     res.writeHead(429, { 'content-type': 'text/plain' }).end('Too many concurrent connections')
                     return
                 }
                 activeConnections.inc()
             }
 
             const mcpServer = buildServer(apiToken ? { apiToken } : undefined)
+            let sessionEstablished = false
             const transport = new StreamableHTTPServerTransport({
                 sessionIdGenerator: () => randomUUID(),
                 onsessioninitialized: (id) => {
+                    sessionEstablished = true
                     mcpServer.setSessionId(id)
                     logger.info({ session: sessionFingerprint(id) }, 'Session initialized')
                     sessions.set(id, {
                         transport,
                         apiToken,
                         apiTokenDigest: apiToken ? digest(apiToken) : undefined,
+                        used: false,
                         lastActivity: Date.now(),
                         close: cleanup,
                     })
@@ -274,7 +309,7 @@ export async function startHttpServer(options: StartHttpServerOptions = {}) {
             const cleanup = async () => {
                 if (cleaned) return
                 cleaned = true
-                activeSessions.dec()
+                if (sessionEstablished) activeSessions.dec()
                 if (apiToken && rateLimiter) {
                     rateLimiter.connections.release(apiToken)
                     activeConnections.dec()
@@ -298,6 +333,17 @@ export async function startHttpServer(options: StartHttpServerOptions = {}) {
                 void cleanup()
             }
 
+            /*
+             * An initialize request that never produces a session — a malformed
+             * body the SDK rejects, a client that hangs up, a transport error —
+             * leaves the transport unopened, so `onclose` never fires and the
+             * idle sweeper never sees it either. Without this the slot it took
+             * above was held until the process restarted.
+             */
+            res.on('close', () => {
+                if (!sessionEstablished) void cleanup()
+            })
+
             await mcpServer.connect(transport)
             await transport.handleRequest(req, res, body)
             return
@@ -339,6 +385,7 @@ export async function startHttpServer(options: StartHttpServerOptions = {}) {
                 }
             }
 
+            session.used = true
             session.lastActivity = Date.now()
             await session.transport.handleRequest(req, res, body)
             return
@@ -483,11 +530,16 @@ export async function startHttpServer(options: StartHttpServerOptions = {}) {
     // Idle session cleanup
     const sessionIdleTimeoutMs = options.sessionIdleTimeoutMs ?? 10 * 60 * 1000
     const sessionCheckIntervalMs = options.sessionCheckIntervalMs ?? 60 * 1000
+    // A session that never made a call gets a much shorter leash than one doing
+    // real work — it is holding a connection slot for nothing. Never longer
+    // than the ordinary idle timeout, whatever the caller passes.
+    const unusedSessionTimeoutMs = Math.min(options.unusedSessionTimeoutMs ?? 30 * 1000, sessionIdleTimeoutMs)
     const idleCheckInterval = setInterval(() => {
         const now = Date.now()
         for (const [sessionId, session] of sessions.entries()) {
-            if (now - session.lastActivity > sessionIdleTimeoutMs) {
-                logger.info({ session: sessionFingerprint(sessionId) }, 'Closing idle session')
+            const timeout = session.used ? sessionIdleTimeoutMs : unusedSessionTimeoutMs
+            if (now - session.lastActivity > timeout) {
+                logger.info({ session: sessionFingerprint(sessionId), used: session.used }, 'Closing idle session')
                 void session.close()
             }
         }

src/ratelimit/connectionCounter.ts

@@ -2,7 +2,7 @@
  * Tracks concurrent connections per key with a configurable limit.
  */
 export class ConnectionCounter {
-    private readonly maxConnections: number
+    readonly maxConnections: number
     private readonly counts = new Map<string, number>()
 
     constructor(maxConnections: number) {
@@ -24,4 +24,9 @@ export class ConnectionCounter {
             this.counts.set(key, current - 1)
         }
     }
+
+    /** Slots currently held for a key — for diagnosing an exhausted pool. */
+    active(key: string): number {
+        return this.counts.get(key) ?? 0
+    }
 }