Skip to content

fix(http): detach per-session loggers from the shared file transport on stop() - #412

Merged
debugmcpdev merged 1 commit into
mainfrom
fix/404-logger-detach
Aug 22, 2026
Merged

fix(http): detach per-session loggers from the shared file transport on stop()#412
debugmcpdev merged 1 commit into
mainfrom
fix/404-logger-detach

Conversation

@debugmcpdev

Copy link
Copy Markdown
Collaborator

Fixes #404

In Streamable HTTP mode every MCP session builds a full DebugMcpServer, whose DI container creates a winston logger piped into the process-lifetime shared file transport (fileTransportCache, one transport per path — issue #121's rotation-correctness invariant). Nothing ever unpiped: stop() didn't touch the logger, and logger.close() is forbidden because it would close the shared transport for every logger. So the shared transport accumulated one pipe edge + listener set + retained logger per HTTP session, forever — exactly the churn the MCP_HTTP_STALE_SESSION_MS reaper (#337) exists for, and it couldn't reclaim this.

Change

  • logger.ts: a WeakMap<logger, transport> records which shared transport each createLogger call attached; new detachSharedFileTransport(logger) removes that transport from the logger (unpiping it) without closing it.
  • The subtle part: winston-transport registers once('unpipe', src => { if (src === this.parent) { this.parent = null; this.close(); } }) where parent is the first logger that piped the transport — a plain logger.remove() from that logger closes the shared file stream for everyone still using it. The detach shadows transport.close with an own undefined property for the duration of the remove (the handler's if (this.close) is then falsy) and restores it in finally.
  • Dependencies gains optional disposeLogger?: () => void; createProductionDependencies returns () => detachSharedFileTransport(logger) (the closure keeps the concrete winston type — ILogger in shared never widens).
  • DebugMcpServer.stop() calls this.disposeLogger?.() last, after the final "Debug MCP Server stopped" line, so that line still reaches the file.
  • logger.close() remains forbidden; the shared transport stays open and cached for the process lifetime, as documented.

Tests (TDD, watched fail first)

  • New logger-detach.test.ts runs against real winston (no module mock — the leak lives in winston's pipe mechanics):
    • 5 churning create+detach cycles against one shared transport: listener counts (unpipe/error) don't grow, cache identity holds.
    • The regression that shaped the design: detaching the first attacher must NOT close the shared transport (close spy) and the surviving logger still logs.
    • No-op for loggers without a file transport.
  • dependencies.test.ts: disposeLogger wired to detachSharedFileTransport with the container's logger (its logger.js mock factory gained the new export).
  • server-lifecycle.test.ts: stop() invokes the disposer exactly once.
  • Full npm test green, npm run lint clean.

Not touched (noted in the issue): the deprecated SSE transport's session map has no stale reaper, and the module-level defaultLogger overwrite retains only the newest logger — neither is a growth leak.

🤖 Generated with Claude Code

…on stop()

Every Streamable HTTP session's DebugMcpServer piped its logger into the
process-lifetime shared file transport and nothing ever unpiped — one
listener set + retained logger per session, forever (issue #404).

createLogger now records the attached shared transport in a WeakMap;
detachSharedFileTransport(logger) removes it without closing it —
shadowing transport.close during the remove because winston-transport's
close-on-unpipe fires when the FIRST attacher unpipes and would close
the shared file stream for everyone. The DI container exposes the detach
as Dependencies.disposeLogger and DebugMcpServer.stop() calls it last.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@codecov

codecov Bot commented Aug 22, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 94.11765% with 1 line in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/utils/logger.ts 92.85% 1 Missing ⚠️

📢 Thoughts on this report? Let us know!

@debugmcpdev
debugmcpdev merged commit 0ef2b9a into main Aug 22, 2026
9 checks passed
@debugmcpdev
debugmcpdev deleted the fix/404-logger-detach branch August 22, 2026 03:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

leak(http): per-session logger piped into process-lifetime shared transport, never detached on stop()

2 participants