Skip to content
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
123 changes: 115 additions & 8 deletions src/server.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,11 +9,10 @@ import {
type ServerResult,
} from '@modelcontextprotocol/sdk/types.js';
import { spawn } from 'node:child_process';
import { existsSync } from 'node:fs';
import { existsSync, mkdirSync, readFileSync, writeFileSync } from 'node:fs';
import { homedir } from 'node:os';
import { join, resolve as pathResolve } from 'node:path';
import * as path from 'path';
import { readFileSync } from 'node:fs';

// Server version - update this when releasing new versions
const SERVER_VERSION = "1.10.12";
Expand Down Expand Up @@ -75,24 +74,132 @@ interface ClaudeCliResponse {
}

/**
* Session mapping: parentSessionId -> claudeCliSessionId
* Limited to MAX_SESSIONS to prevent memory leaks in long-running servers
* Session entry with timestamp for expiration
*/
interface SessionEntry {
claudeSessionId: string;
updatedAt: string;
}

/**
* Session storage configuration
*/
const MAX_SESSIONS = 1000;
const sessionMap = new Map<string, string>();
const SESSION_TTL_MS = 24 * 60 * 60 * 1000; // 24 hours
const SESSION_DIR = join(homedir(), '.config', 'claude-code-mcp');
const SESSION_FILE = join(SESSION_DIR, 'sessions.json');

/**
* In-memory session cache, loaded from file on startup
*/
let sessionMap: Map<string, SessionEntry> = new Map();

/**
* Ensure the session directory exists
*/
function ensureSessionDir(): void {
if (!existsSync(SESSION_DIR)) {
mkdirSync(SESSION_DIR, { recursive: true });
debugLog(`[Debug] Created session directory: ${SESSION_DIR}`);
}
}

/**
* Load sessions from file into memory
*/
function loadSessions(): void {
try {
if (existsSync(SESSION_FILE)) {
const data = readFileSync(SESSION_FILE, 'utf-8');
const parsed = JSON.parse(data);
sessionMap = new Map(Object.entries(parsed));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because sessions.json is effectively external input, Object.entries(parsed) can populate sessionMap with values missing a valid updatedAt/claudeSessionId; then new Date(entry.updatedAt).getTime() becomes NaN and TTL expiry won’t work (or resumes can silently fail). Consider validating/coercing loaded entries before assigning to sessionMap.

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎

debugLog(`[Debug] Loaded ${sessionMap.size} sessions from ${SESSION_FILE}`);

// Clean up expired sessions on load
cleanExpiredSessions();
}
} catch (e) {
debugLog('[Debug] Failed to load sessions file, starting fresh:', e);
sessionMap = new Map();
}
Comment on lines +121 to +124

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Failures in loading the session file are logged using debugLog, which is conditional on the MCP_CLAUDE_DEBUG environment variable. A failure to load sessions is a significant event that leads to the loss of all session continuity. This should be logged as a standard error to ensure it's always visible, regardless of debug settings.

Suggested change
} catch (e) {
debugLog('[Debug] Failed to load sessions file, starting fresh:', e);
sessionMap = new Map();
}
} catch (e) {
console.error('[Error] Failed to load sessions file, starting fresh:', e);
sessionMap = new Map();
}

}

/**
* Save sessions from memory to file
*/
function saveSessions(): void {
try {
ensureSessionDir();
const data = Object.fromEntries(sessionMap);
writeFileSync(SESSION_FILE, JSON.stringify(data, null, 2));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Since saveSessions() overwrites sessions.json directly, concurrent MCP processes (or a crash mid-write) can leave a partially-written file that resets sessions on the next startup. Consider making the persistence write atomic to reduce the chance of JSON corruption.

Fix This in Augment

🤖 Was this useful? React with 👍 or 👎

debugLog(`[Debug] Saved ${sessionMap.size} sessions to ${SESSION_FILE}`);
} catch (e) {
debugLog('[Debug] Failed to save sessions file:', e);
}
Comment on lines +136 to +138

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

Failures in saving the session file are logged using debugLog, which is conditional. A failure to save sessions can lead to data inconsistency between memory and disk and should be logged as a standard error to ensure the issue is always visible.

Suggested change
} catch (e) {
debugLog('[Debug] Failed to save sessions file:', e);
}
} catch (e) {
console.error('[Error] Failed to save sessions file:', e);
}

}
Comment on lines +110 to +139

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

critical

The current implementation of loading from and saving to sessions.json is not safe for concurrent execution. The pull request description mentions that each npx call spawns a new process. If two such processes run concurrently, they can create a race condition:

  1. Process A reads sessions.json.
  2. Process B reads sessions.json.
  3. Process A modifies its in-memory session map and writes the result to sessions.json.
  4. Process B modifies its in-memory session map and writes its result to sessions.json, overwriting the changes from Process A.

This will lead to session data loss. To prevent this, you should implement a file locking mechanism around the read-modify-write cycle to ensure atomic updates to the session file. Libraries like proper-lockfile are well-suited for this purpose.

Comment on lines +130 to +139

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

high

The use of writeFileSync makes this a synchronous, blocking I/O operation. This function is called frequently (on every session creation/update via setSessionMapping, on expiration checks in getSessionMapping, and during cleanup). In a long-running server context, blocking the Node.js event loop for file I/O will cause significant performance degradation.

All file I/O should be performed asynchronously using fs.promises. This would require this function and its callers to become async and use await. Additionally, you might consider "debouncing" calls to saveSessions to avoid inefficiently writing to disk on every single modification.


/**
* Store a session mapping with LRU-style eviction
* Remove sessions older than TTL
*/
function cleanExpiredSessions(): void {
const now = Date.now();
let cleaned = 0;

for (const [parentId, entry] of sessionMap) {
const age = now - new Date(entry.updatedAt).getTime();
if (age > SESSION_TTL_MS) {
sessionMap.delete(parentId);
cleaned++;
}
}

if (cleaned > 0) {
debugLog(`[Debug] Cleaned ${cleaned} expired sessions`);
saveSessions();
}
}

/**
* Get a session mapping
*/
function getSessionMapping(parentId: string): string | undefined {
const entry = sessionMap.get(parentId);
if (entry) {
// Check if expired
const age = Date.now() - new Date(entry.updatedAt).getTime();
if (age > SESSION_TTL_MS) {
sessionMap.delete(parentId);
saveSessions();
return undefined;
}
return entry.claudeSessionId;
}
return undefined;
}

/**
* Store a session mapping with LRU-style eviction and file persistence
*/
Comment on lines +181 to 182

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

The comment describes "LRU-style eviction", but the implementation is actually FIFO (First-In, First-Out), as it removes the oldest entry by insertion order (sessionMap.keys().next().value). A true LRU cache evicts the least recently used item. To avoid confusion, please update the comment to accurately describe the FIFO behavior.

Suggested change
* Store a session mapping with LRU-style eviction and file persistence
*/
* Store a session mapping with FIFO-style eviction and file persistence

function setSessionMapping(parentId: string, claudeId: string): void {
// LRU eviction if at capacity
if (sessionMap.size >= MAX_SESSIONS) {
// Remove oldest entry (first inserted)
const firstKey = sessionMap.keys().next().value;
if (firstKey) sessionMap.delete(firstKey);
}
sessionMap.set(parentId, claudeId);

sessionMap.set(parentId, {
claudeSessionId: claudeId,
updatedAt: new Date().toISOString()
});

// Persist to file
saveSessions();
}

// Load sessions on module initialization
loadSessions();

/**
* Determines the Claude CLI command/path based on the following precedence:
* 1. An absolute path specified in the `CLAUDE_CLI_NAME` environment variable.
Expand Down Expand Up @@ -432,7 +539,7 @@ export class ClaudeCodeServer {
const claudeProcessArgs: string[] = ['--dangerously-skip-permissions'];

if (!stateless && parentSessionId) {
const existingClaudeSessionId = sessionMap.get(parentSessionId);
const existingClaudeSessionId = getSessionMapping(parentSessionId);

if (existingClaudeSessionId) {
debugLog(`[Debug] Resuming Claude CLI session: ${existingClaudeSessionId} for parent session: ${parentSessionId}`);
Expand Down