OTEL: Session-related telemetry and code quality issues

Context

These issues were identified during the OTEL MCP semantic conventions review (parent issue: #3399). They are all related to session management code that is expected to undergo significant redesign. Tracking them here so we can check which ones get resolved by the session refactor and which need separate fixes.

Issues

StreamableSession.Disconnect() has a data race (High)

File: pkg/transport/session/streamable_session.go:46-77

The disconnected field is read and written without any synchronization. Concurrent calls to Disconnect() and SendMessage()/SendResponse() can race, potentially causing a double-close panic on channels. Compare with SSESession which correctly uses a mutex.

Fix: Add mutex synchronization matching the SSESession pattern.

"terminated" magic string used across packages (Low)

Files: pkg/transport/session/manager.go:286,308, pkg/vmcp/server/session_adapter.go:105,144

The string "terminated" is used as a metadata key across two packages. A typo would cause silent bugs.

Fix: Define MetadataKeyTerminated constant in the session package.

Potential double-callback race in cleanupExpiredOnce (Low)

File: pkg/transport/session/manager.go

cleanupExpiredOnce and concurrent TerminateSession can fire a session's end callback twice. The safeCallback panic recovery prevents crashes but duplicate metric recordings can occur.

Fix: Use an atomic flag per session to ensure the callback fires at most once.

mcp.server.session.duration metric not implemented (Medium)

Files: pkg/vmcp/server/server.go, pkg/transport/session/manager.go

The OTEL MCP semconv defines mcp.server.session.duration as a recommended histogram metric. The session manager already tracks creation timestamps, so computing duration at termination/expiry is straightforward.

Fix: Add a Float64Histogram metric in the session end callback, recording time.Since(session.CreatedAt()).

Expired-session callbacks silently skip non-LocalStorage backends (Medium)

File: pkg/transport/session/manager.go:299-314

The cleanupExpiredOnce type-asserts *LocalStorage to iterate. Non-local backends silently skip callbacks. Document the limitation or add ListExpired to the Storage interface.

Validate touches terminated sessions, extending TTL (Low)

File: pkg/vmcp/server/session_adapter.go:92

Get() extends the session TTL even for terminated sessions, preventing cleanup. Terminated sessions should expire quickly rather than being kept alive by validation probes.

Notes

• These should be revisited after the session management redesign
• Some may be resolved as part of the refactor; others may need separate fixes
• The StreamableSession.Disconnect() data race is the highest severity item (potential panic)

Start with pkg/transport/session/streamable_session.go:46-77 and compare its synchronization with SSESession. Then review the listed manager.go, session_adapter.go, and server.go locations, including callback, metadata, metric, storage, and TTL behavior. Done means each issue is fixed or explicitly documented after the planned session redesign.