fix(mcp): invalidate cached sessions when DynamicHeaders change#27
Merged
fix(mcp): invalidate cached sessions when DynamicHeaders change#27
Conversation
The ClientManager cached MCP sessions by server name but never checked if the authentication headers had changed. When Safari restarts with different credentials (e.g. SAFARI_FLASHDUTY_MCP_APP_KEY override), the runner kept reusing the stale session with old headers, causing 401 Unauthorized errors on tool calls. Now we hash both static and dynamic headers at session creation time and compare on reuse. If headers differ, the stale session is closed and a new one is created with the current credentials. Also adds masked auth_key (first 6 chars) to session lifecycle logs for easier debugging without leaking secrets. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- Replace SHA-256 hash with sorted key=value concatenation for header comparison — simpler, debuggable, and avoids ambiguous boundaries - Move cache key computation outside mutex to minimize critical section - Remove redundant comments on self-documenting functions Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
- G706 (log injection): exclude globally — slog structured logging is inherently safe against injection - G204 (subprocess with variable): add nolint for intentional exec calls (MCP stdio, bash workspace, rg grep) - G118 (context cancel): nolint — cancel is stored in runningTask map - S1017: use strings.TrimPrefix instead of if+slice - QF1012: use fmt.Fprintf instead of WriteString(fmt.Sprintf(...)) - noctx: use exec.CommandContext for system info commands - prealloc: preallocate grep cmdArgs slice Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
ClientManager.GetSessionreused stale sessions even whenDynamicHeaders(auth credentials) changed, causing 401 Unauthorized on MCP tool callsauth_key(first 6 chars) to session lifecycle logs for debugging without leaking secretsTest plan
SAFARI_FLASHDUTY_MCP_APP_KEYon Safari, verify runner logs showmcp headers changed, invalidating cached sessionon first call after restartauth_key=<first6>***appears in runner logs for session creationquery_incidents) succeed with production key🤖 Generated with Claude Code