-
-
Notifications
You must be signed in to change notification settings - Fork 4
Fix MCP server pointer error and integrate dashboard toggle #73
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
- Fix nil pointer dereference in MCP logging middleware when session is nil - Add settings table to database schema for persisting MCP enabled state - Add GetSetting, SetSetting, GetAllSettings methods to storage layer - Integrate MCP server into main HTTP server at /mcp endpoint - Add API endpoints: GET/PUT /api/settings/mcp for MCP toggle control - Add settings panel UI with MCP enable/disable toggle in dashboard - MCP is disabled by default and can be hot-toggled without restart - Support OAuth2 authentication when mcp-oauth flags are provided - Update logging to use zerolog consistently throughout main.go The MCP server is now accessible at /mcp when enabled from the dashboard settings panel. Users can toggle MCP on/off without restarting the app.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull request overview
This pull request adds a settings panel UI and integrates the MCP (Model Context Protocol) server into the main HTTP server with hot-toggle capability. The changes fix a nil pointer dereference in MCP logging and add persistent storage for the MCP enabled state.
Key changes:
- Fixed nil pointer error in MCP server logging middleware by checking if session exists before accessing
- Added database table and storage methods for persisting settings (mcp_enabled state)
- Integrated MCP server at /mcp endpoint within the main HTTP server with dynamic enable/disable based on settings
- Added settings panel UI with MCP toggle control, accessible from dashboard
Reviewed changes
Copilot reviewed 9 out of 9 changed files in this pull request and generated 7 comments.
Show a summary per file
| File | Description |
|---|---|
| src/components/settings/SettingsPanel.vue | New Vue component providing settings UI with MCP enable/disable toggle |
| src/assets/base.css | Added primary-soft color variables for light and dark themes |
| src/App.vue | Integrated settings panel and added settings button to navigation, renamed btn-theme to btn-icon |
| main.go | Updated logging to use zerolog consistently, added OAuth configuration for integrated MCP mode |
| internal/storage/sqlite_no_cgo.go | Added settings table schema to SQLite initialization |
| internal/storage/sqlite_cgo.go | Added settings table schema to SQLite initialization |
| internal/storage/common.go | Added GetSetting, SetSetting, GetAllSettings methods for settings persistence |
| internal/mcp/server.go | Fixed nil pointer dereference in logging middleware, added Handler method for mounting MCP on existing server |
| internal/api/server.go | Added MCP integration with hot-toggle support, new API endpoints for settings, updated logging to use zerolog |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
| const toggleMCP = async () => { | ||
| saving.value = true; | ||
| error.value = null; | ||
| success.value = null; | ||
| const newValue = !settings.value.mcp_enabled; | ||
| try { | ||
| const response = await fetch("/api/settings/mcp", { | ||
| method: "PUT", | ||
| headers: { | ||
| "Content-Type": "application/json", | ||
| }, | ||
| body: JSON.stringify({ enabled: newValue }), | ||
| }); | ||
| if (!response.ok) { | ||
| const data = await response.json(); | ||
| throw new Error(data.error || "Failed to update setting"); | ||
| } | ||
| const data = await response.json(); | ||
| settings.value.mcp_enabled = data.enabled; | ||
| success.value = data.message; | ||
| // Clear success message after 3 seconds | ||
| setTimeout(() => { | ||
| success.value = null; | ||
| }, 3000); | ||
| } catch (err) { | ||
| error.value = err.message; | ||
| } finally { | ||
| saving.value = false; | ||
| } | ||
| }; |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If the request fails, settings.value.mcp_enabled is not reverted to its original state. The toggle UI might appear to be in the new state while the backend still has the old state. Consider adding an optimistic update with rollback on error, or ensure the UI reflects the actual state by re-fetching settings after an error.
| var oauthCfg *oauth.Config | ||
| if mcpOAuthEnabled { | ||
| var scopes []string | ||
| if mcpOAuthScopes != "" { | ||
| for _, s := range strings.Split(mcpOAuthScopes, ",") { | ||
| scopes = append(scopes, strings.TrimSpace(s)) | ||
| } | ||
| } | ||
|
|
||
| var resourceServerURL, audience string | ||
| if mcpOAuthAudience != "" { | ||
| resourceServerURL = mcpOAuthAudience | ||
| audience = mcpOAuthAudience | ||
| } else { | ||
| resourceServerURL = fmt.Sprintf("http://localhost:%d/mcp", cfg.Server.Port) | ||
| audience = resourceServerURL | ||
| } | ||
|
|
||
| oauthCfg = &oauth.Config{ | ||
| Enabled: true, | ||
| Issuer: mcpOAuthIssuer, | ||
| Audience: audience, | ||
| ClientID: mcpOAuthClientID, | ||
| ClientSecret: mcpOAuthClientSecret, | ||
| RequiredScopes: scopes, | ||
| IntrospectionEndpoint: mcpOAuthIntrospection, | ||
| ResourceServerURL: resourceServerURL, | ||
| ResourceName: mcpOAuthResourceName, | ||
| InsecureSkipVerify: mcpOAuthInsecure, | ||
| } |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The OAuth configuration code is duplicated between lines 213-246 (for standalone MCP mode) and lines 264-293 (for integrated mode). Consider extracting this into a helper function like buildOAuthConfig(mcpOAuthEnabled bool, ...) *oauth.Config to eliminate duplication and ensure consistency.
| func (s *Server) RunHTTP(ctx context.Context, addr string, oauthCfg *oauth.Config) error { | ||
| // Handler returns an http.Handler for the MCP server that can be mounted | ||
| // on an existing HTTP server. The basePath is used for OAuth metadata URLs. | ||
| func (s *Server) Handler(basePath string, oauthCfg *oauth.Config) (http.Handler, error) { |
Copilot
AI
Dec 18, 2025
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The basePath parameter is documented in the comment but never used in the function body. The OAuth metadata URL is constructed from oauthCfg.ResourceServerURL instead. Either remove this unused parameter or use it to construct the metadata URL if that was the intent.
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
Release-As: 1.4.1
The MCP server is now accessible at /mcp when enabled from the dashboard settings panel. Users can toggle MCP on/off without restarting the app.