fix: implement sliding window expiration for MCP sessions#39
Conversation
ChenNima
left a comment
There was a problem hiding this comment.
Thanks for implementing sliding window expiration — this is a real improvement over the fixed setTimeout. The core idea (activity tracking + periodic cleanup) is solid.
Two things to address before merging:
-
Rebase onto main after #37 is merged — this PR includes all of #37's changes (project filtering, CLAUDE.md updates, doc changes). Once #37 lands, please rebase so this PR only contains the session expiration changes. Right now there will be merge conflicts.
-
Move Prisma query out of route.ts — see inline comment.
|
Hi @zhuangzhuangzhou, this PR conflicts with #37. Please resolve the conflict before it can be merged. |
- Replace fixed 30-minute timeout with activity-based expiration - Add lastActivity tracking to session metadata - Update activity timestamp on each request via touchSession() - Implement global setInterval cleanup (every 5 minutes) - Sessions expire after 30 minutes of inactivity (not creation time) - Add comprehensive test suite for session lifecycle - Update documentation (ARCHITECTURE.md, MCP_TOOLS.md, skill docs) Fixes frequent "Session not found. Please reinitialize." errors 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Use vi.hoisted() to create mockTransport, making it available in mock factory - Change mock constructor from arrow function to regular function - Use dynamic imports in each test to avoid binding issues - Update cleanup test to verify behavior (404 response) instead of mock assertions This fixes CI test failures caused by mock binding issues with vi.resetModules() and fake timers. All 8 tests now pass reliably. 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Fix mock constructor syntax in MCP session tests (use function instead of arrow function) - Add automatic retry mechanism (max 3 retries) on 404 session expired - Detect HTTP status code and reinitialize session on failure - Resolve user pain point: session expiry no longer interrupts agent conversations 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
- Add getProjectUuidsByGroup() to project.service.ts - Replace direct Prisma query in route.ts with service call - Add comment about setInterval assuming persistent Node.js process Addresses PR review feedback: - Move Prisma operations to service layer per project convention - Document setInterval limitations in serverless/edge environments 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude Sonnet 4.5 <noreply@anthropic.com>
592a7fc to
05a4a12
Compare
ChenNima
left a comment
There was a problem hiding this comment.
Thanks for the updates — sliding window expiration, service layer refactor, and auto-reconnect in chorus-api.sh all look good. Tested locally: all 1090 unit tests pass, MCP endpoint behaves correctly, and chorus-api.sh works end-to-end. Just one thing to fix, then ready to merge.
Fixes frequent "Session not found. Please reinitialize." errors