Refresh fixes #5
Merged
Conversation
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
![baz-reviewer[bot]](https://avatars.githubusercontent.com/in/933528?s=60&v=4){kind=small}
| - Helpful for debugging | ||
|
|
||
| ## MCP Client Configuration | ||
| **Note**: The `apollo_key` can reference environment variables using `${DC_API_KEY}` syntax. |
There was a problem hiding this comment.
Should apollo_key be apollo-key to match the configuration examples above? The config uses hyphens but this note uses underscores.
Suggested change
| **Note**: The `apollo_key` can reference environment variables using `${DC_API_KEY}` syntax. | |
| **Note**: The `apollo-key` can reference environment variables using `${DC_API_KEY}` syntax. |
Finding type: Naming and Typos
Comment on lines
190
to
194
| config_manager.update_auth_token(&token_response.access_token) | ||
| .map_err(|e| { | ||
| error!("Failed to write refreshed token to config file: {}", e); | ||
| e | ||
| })?; |
There was a problem hiding this comment.
The function now fails completely if config writing fails, even when token refresh succeeded. Should config write failures be non-fatal since the primary operation (token refresh) succeeded?
Suggested change
| config_manager.update_auth_token(&token_response.access_token) | |
| .map_err(|e| { | |
| error!("Failed to write refreshed token to config file: {}", e); | |
| e | |
| })?; | |
| if let Err(e) = config_manager.update_auth_token(&token_response.access_token) { | |
| error!("Failed to write refreshed token to config file: {}", e); | |
| } else { | |
| info!("✅ Refreshed token written to config file"); | |
| } |
Finding type: Logical Bugs
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.
Generated description
Below is a concise technical summary of the changes proposed in this PR:
graph LR main_("main"):::modified initialize_with_token_refresh_("initialize_with_token_refresh"):::modified TokenManager_new_("TokenManager.new"):::modified TokenManager_set_config_manager_("TokenManager.set_config_manager"):::modified TokenManager_set_headers_("TokenManager.set_headers"):::added TokenManager_get_valid_token_("TokenManager.get_valid_token"):::modified TokenManager_refresh_access_token_("TokenManager.refresh_access_token"):::modified ConfigManager_update_auth_token_("ConfigManager.update_auth_token"):::modified TokenManager_headers_("TokenManager.headers"):::added Starting_start_("Starting.start"):::modified Running_("Running"):::modified Running_call_tool_("Running.call_tool"):::modified Running_headers_("Running.headers"):::added main_ -- "Passes shared headers for token refresh and header updates" --> initialize_with_token_refresh_ initialize_with_token_refresh_ -- "Creates TokenManager with support for shared headers injection" --> TokenManager_new_ initialize_with_token_refresh_ -- "Injects config manager for token persistence in TokenManager" --> TokenManager_set_config_manager_ initialize_with_token_refresh_ -- "Injects shared headers for automatic token authorization updates" --> TokenManager_set_headers_ initialize_with_token_refresh_ -- "Obtains fresh token, triggers shared headers update" --> TokenManager_get_valid_token_ TokenManager_refresh_access_token_ -- "Writes refreshed token to config file for persistence" --> ConfigManager_update_auth_token_ TokenManager_refresh_access_token_ -- "Updates shared Authorization header with refreshed token asynchronously" --> TokenManager_headers_ Starting_start_ -- "Creates Running state with shared headers for auth propagation" --> Running_ Running_call_tool_ -- "Reads shared headers to propagate auth tokens in requests" --> Running_headers_ classDef added stroke:#15AA7A classDef removed stroke:#CD5270 classDef modified stroke:#EDAC4C linkStyle default stroke:#CBD5E1,font-size:13pxImplements a shared, mutable header map (
Arc<RwLock>) to dynamically update authentication tokens across thedc-mcp-serverwithout re-reading configuration. Ensures that all outgoing GraphQL requests utilize the most recently refreshed token, improving the reliability of the Model Context Protocol (MCP) server's interaction with GraphQL APIs.Latest Contributors(2)