mirror of
https://github.com/czlonkowski/n8n-mcp.git
synced 2026-01-30 06:22:04 +00:00
Revert to v2.18.10 - Remove session persistence (v2.19.0-v2.19.5) (#322)
After 5 consecutive hotfix attempts, session persistence has proven architecturally incompatible with the MCP SDK. Rolling back to last known stable version. ## Removed - 16 new files (session types, docs, tests, planning docs) - 1,100+ lines of session persistence code - Session restoration hooks and lifecycle events - Retry policy and warm-start implementations ## Restored - Stable v2.18.10 codebase - Library export fields (from PR #310) - All core MCP functionality ## Breaking Changes - Session persistence APIs removed - onSessionNotFound hook removed - Session lifecycle events removed This reverts commitsfe13091through1d34ad8. Restores commit4566253(v2.18.10, PR #310). 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-authored-by: Claude <noreply@anthropic.com>
This commit is contained in:
Romuald Członkowski
committed by
GitHub
GitHub
parent
fe1309151a
commit
8d20c64f5c
@@ -1,83 +0,0 @@
|
||||
# Multi-App Integration Guide
|
||||
|
||||
This guide explains how session restoration works in n8n-mcp for multi-tenant deployments.
|
||||
|
||||
## Session Restoration: Warm Start Pattern
|
||||
|
||||
When a container restarts, existing client sessions are lost. The warm start pattern allows clients to seamlessly restore sessions without manual intervention.
|
||||
|
||||
### How It Works
|
||||
|
||||
1. **Client sends request** with existing session ID after restart
|
||||
2. **Server detects** unknown session ID
|
||||
3. **Restoration hook** is called to load session context from your database
|
||||
4. **New session created** using restored context
|
||||
5. **Current request handled** immediately through new transport
|
||||
6. **Client receives** standard MCP error `-32000` (Server not initialized)
|
||||
7. **Client auto-retries** with initialize request on same connection
|
||||
8. **Session fully restored** and client continues normally
|
||||
|
||||
### Key Features
|
||||
|
||||
- **Zero client changes**: Standard MCP clients auto-retry on -32000
|
||||
- **Single HTTP round-trip**: No extra network requests needed
|
||||
- **Concurrent-safe**: Idempotency guards prevent duplicate restoration
|
||||
- **Automatic cleanup**: Failed restorations clean up resources automatically
|
||||
|
||||
### Implementation
|
||||
|
||||
```typescript
|
||||
import { SingleSessionHTTPServer } from 'n8n-mcp';
|
||||
|
||||
const server = new SingleSessionHTTPServer({
|
||||
// Hook to load session context from your storage
|
||||
onSessionNotFound: async (sessionId) => {
|
||||
const session = await database.loadSession(sessionId);
|
||||
if (!session || session.expired) {
|
||||
return null; // Reject restoration
|
||||
}
|
||||
return session.instanceContext; // Restore session
|
||||
},
|
||||
|
||||
// Optional: Configure timeouts and retries
|
||||
sessionRestorationTimeout: 5000, // 5 seconds (default)
|
||||
sessionRestorationRetries: 2, // Retry on transient failures
|
||||
sessionRestorationRetryDelay: 100 // Delay between retries
|
||||
});
|
||||
```
|
||||
|
||||
### Session Lifecycle Events
|
||||
|
||||
Track session restoration for metrics and debugging:
|
||||
|
||||
```typescript
|
||||
const server = new SingleSessionHTTPServer({
|
||||
sessionEvents: {
|
||||
onSessionRestored: (sessionId, context) => {
|
||||
console.log(`Session ${sessionId} restored`);
|
||||
metrics.increment('session.restored');
|
||||
}
|
||||
}
|
||||
});
|
||||
```
|
||||
|
||||
### Error Handling
|
||||
|
||||
The restoration hook can return three outcomes:
|
||||
|
||||
- **Return context**: Session is restored successfully
|
||||
- **Return null/undefined**: Session is rejected (client gets 400 Bad Request)
|
||||
- **Throw error**: Restoration failed (client gets 500 Internal Server Error)
|
||||
|
||||
Timeout errors are never retried (already took too long).
|
||||
|
||||
### Concurrency Safety
|
||||
|
||||
Multiple concurrent requests for the same session ID are handled safely:
|
||||
|
||||
- First request triggers restoration
|
||||
- Subsequent requests reuse the restored session
|
||||
- No duplicate session creation
|
||||
- No race conditions
|
||||
|
||||
This ensures correct behavior even under high load or network retries.
|
||||
@@ -1,180 +0,0 @@
|
||||
# Bug Fix: onSessionCreated Event Not Firing (v2.19.0)
|
||||
|
||||
## Summary
|
||||
|
||||
Fixed critical bug where `onSessionCreated` lifecycle event was never emitted for sessions created during the standard MCP initialize flow, completely breaking session persistence functionality.
|
||||
|
||||
## Impact
|
||||
|
||||
- **Severity**: Critical
|
||||
- **Affected Version**: v2.19.0
|
||||
- **Component**: Session Persistence (Phase 3)
|
||||
- **Status**: ✅ Fixed
|
||||
|
||||
## Root Cause
|
||||
|
||||
The `handleRequest()` method in `http-server-single-session.ts` had two different paths for session creation:
|
||||
|
||||
1. **Standard initialize flow** (lines 868-943): Created session inline but **did not emit** `onSessionCreated` event
|
||||
2. **Manual restoration flow** (line 1048): Called `createSession()` which **correctly emitted** the event
|
||||
|
||||
This inconsistency meant that:
|
||||
- New sessions during normal operation were **never saved to database**
|
||||
- Only manually restored sessions triggered the save event
|
||||
- Session persistence was completely broken for new sessions
|
||||
- Container restarts caused all sessions to be lost
|
||||
|
||||
## The Fix
|
||||
|
||||
### Location
|
||||
- **File**: `src/http-server-single-session.ts`
|
||||
- **Method**: `handleRequest()`
|
||||
- **Line**: After line 943 (`await server.connect(transport);`)
|
||||
|
||||
### Code Change
|
||||
|
||||
Added event emission after successfully connecting server to transport during initialize flow:
|
||||
|
||||
```typescript
|
||||
// Connect the server to the transport BEFORE handling the request
|
||||
logger.info('handleRequest: Connecting server to new transport');
|
||||
await server.connect(transport);
|
||||
|
||||
// Phase 3: Emit onSessionCreated event (REQ-4)
|
||||
// Fire-and-forget: don't await or block session creation
|
||||
this.emitEvent('onSessionCreated', sessionIdToUse, instanceContext).catch(eventErr => {
|
||||
logger.error('Failed to emit onSessionCreated event (non-blocking)', {
|
||||
sessionId: sessionIdToUse,
|
||||
error: eventErr instanceof Error ? eventErr.message : String(eventErr)
|
||||
});
|
||||
});
|
||||
```
|
||||
|
||||
### Why This Works
|
||||
|
||||
1. **Consistent with existing pattern**: Matches the `createSession()` method pattern (line 664)
|
||||
2. **Non-blocking**: Uses `.catch()` to ensure event handler errors don't break session creation
|
||||
3. **Correct timing**: Fires after `server.connect(transport)` succeeds, ensuring session is fully initialized
|
||||
4. **Same parameters**: Passes `sessionId` and `instanceContext` just like the restoration flow
|
||||
|
||||
## Verification
|
||||
|
||||
### Test Results
|
||||
|
||||
Created comprehensive test suite to verify the fix:
|
||||
|
||||
**Test File**: `tests/unit/session/onSessionCreated-event.test.ts`
|
||||
|
||||
**Test Results**:
|
||||
```
|
||||
✓ onSessionCreated Event - Initialize Flow
|
||||
✓ should emit onSessionCreated event when session is created during initialize flow (1594ms)
|
||||
|
||||
Test Files 5 passed (5)
|
||||
Tests 78 passed (78)
|
||||
```
|
||||
|
||||
**Manual Testing**:
|
||||
```typescript
|
||||
const server = new SingleSessionHTTPServer({
|
||||
sessionEvents: {
|
||||
onSessionCreated: async (sessionId, context) => {
|
||||
console.log('✅ Event fired:', sessionId);
|
||||
await saveSessionToDatabase(sessionId, context);
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
// Result: Event fires successfully on initialize!
|
||||
// ✅ Event fired: 40dcc123-46bd-4994-945e-f2dbe60e54c2
|
||||
```
|
||||
|
||||
### Behavior After Fix
|
||||
|
||||
1. **Initialize request** → Session created → `onSessionCreated` event fired → Session saved to database ✅
|
||||
2. **Session restoration** → `createSession()` called → `onSessionCreated` event fired → Session saved to database ✅
|
||||
3. **Manual restoration** → `manuallyRestoreSession()` → Session created → Event fired ✅
|
||||
|
||||
All three paths now correctly emit the event!
|
||||
|
||||
## Backward Compatibility
|
||||
|
||||
✅ **Fully backward compatible**:
|
||||
- No breaking changes to API
|
||||
- Event handler is optional (defaults to no-op)
|
||||
- Non-blocking implementation ensures session creation succeeds even if handler fails
|
||||
- Matches existing behavior of `createSession()` method
|
||||
- All existing tests pass
|
||||
|
||||
## Related Code
|
||||
|
||||
### Event Emission Points
|
||||
|
||||
1. ✅ **Standard initialize flow**: `handleRequest()` at line ~947 (NEW - fixed)
|
||||
2. ✅ **Manual restoration**: `createSession()` at line 664 (EXISTING - working)
|
||||
3. ✅ **Session restoration**: calls `createSession()` indirectly (EXISTING - working)
|
||||
|
||||
### Other Lifecycle Events
|
||||
|
||||
The following events are working correctly:
|
||||
- `onSessionRestored`: Fires when session is restored from database
|
||||
- `onSessionAccessed`: Fires on every request (with throttling recommended)
|
||||
- `onSessionExpired`: Fires before expired session cleanup
|
||||
- `onSessionDeleted`: Fires on manual session deletion
|
||||
|
||||
## Testing Recommendations
|
||||
|
||||
After applying this fix, verify session persistence works:
|
||||
|
||||
```typescript
|
||||
// 1. Start server with session events
|
||||
const engine = new N8NMCPEngine({
|
||||
sessionEvents: {
|
||||
onSessionCreated: async (sessionId, context) => {
|
||||
await database.upsertSession({ sessionId, ...context });
|
||||
}
|
||||
}
|
||||
});
|
||||
|
||||
// 2. Client connects and initializes
|
||||
// 3. Verify session saved to database
|
||||
const sessions = await database.query('SELECT * FROM mcp_sessions');
|
||||
expect(sessions.length).toBeGreaterThan(0);
|
||||
|
||||
// 4. Restart server
|
||||
await engine.shutdown();
|
||||
await engine.start();
|
||||
|
||||
// 5. Client reconnects with old session ID
|
||||
// 6. Verify session restored from database
|
||||
```
|
||||
|
||||
## Impact on n8n-mcp-backend
|
||||
|
||||
This fix **unblocks** the multi-tenant n8n-mcp-backend service that depends on session persistence:
|
||||
|
||||
- ✅ Sessions now persist across container restarts
|
||||
- ✅ Users no longer need to restart Claude Desktop after backend updates
|
||||
- ✅ Session continuity maintained for all users
|
||||
- ✅ Production deployment viable
|
||||
|
||||
## Lessons Learned
|
||||
|
||||
1. **Consistency is critical**: Session creation should follow the same pattern everywhere
|
||||
2. **Event-driven architecture**: Events must fire at all creation points, not just some
|
||||
3. **Testing lifecycle events**: Need integration tests that verify events fire, not just that code runs
|
||||
4. **Documentation**: Clearly document when events should fire and where
|
||||
|
||||
## Files Changed
|
||||
|
||||
- `src/http-server-single-session.ts`: Added event emission (lines 945-952)
|
||||
- `tests/unit/session/onSessionCreated-event.test.ts`: New test file
|
||||
- `tests/integration/session/test-onSessionCreated-event.ts`: Manual verification test
|
||||
|
||||
## Build Status
|
||||
|
||||
- ✅ TypeScript compilation: Success
|
||||
- ✅ Type checking: Success
|
||||
- ✅ All unit tests: 78 passed
|
||||
- ✅ Integration tests: Pass
|
||||
- ✅ Backward compatibility: Verified
|
||||
Reference in New Issue
Block a user