Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 9 additions & 2 deletions internal/admin/dashboard/static/js/dashboard.js
Original file line number Diff line number Diff line change
Expand Up @@ -384,6 +384,12 @@ function dashboard() {
: true;
},

mcpServersPageVisible() {
return typeof this.workflowRuntimeBooleanFlag === "function"
? this.workflowRuntimeBooleanFlag("MCP_ENABLED", true)
: true;
},
Comment thread
SantiagoDePolonia marked this conversation as resolved.

setTheme(t) {
this.theme = t;
localStorage.setItem("gomodel_theme", t);
Expand Down Expand Up @@ -713,8 +719,9 @@ function dashboard() {
) {
requests.push(this.fetchRateLimitsPage());
}
// Fetched on every page, not just mcp-servers: the overview MCP card
// needs the server list to render its connected/total summary.
// Considered on every page, not just mcp-servers: the overview MCP card
// needs the server list to render its connected/total summary. The MCP
// module waits for runtime config and skips the request when disabled.
if (typeof this.fetchMcpServersPage === "function") {
requests.push(this.fetchMcpServersPage());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,6 +81,17 @@ test('Models route starts bounded rendering before Alpine mounts the page', () =
assert.equal(app.page, 'models');
});

test('MCP navigation follows the exposed MCP_ENABLED runtime flag', () => {
const app = loadDashboardApp();
app.workflowRuntimeBooleanFlag = (name, defaultValue) => {
assert.equal(name, 'MCP_ENABLED');
assert.equal(defaultValue, true);
return false;
};

assert.equal(app.mcpServersPageVisible(), false);
});

test('qualifiedModelDisplay keeps provider identity for nested provider model IDs', () => {
const app = loadDashboardApp();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ test("sidebar and main content share the flex layout without manual content offs
assert.doesNotMatch(template, /content-collapsed/);
assert.match(
template,
/href="{{appURL "\/admin\/dashboard\/overview"}}"[\s\S]*<span>Overview<\/span>[\s\S]*href="{{appURL "\/admin\/dashboard\/models"}}"[\s\S]*<span>Models<\/span>[\s\S]*href="{{appURL "\/admin\/dashboard\/audit-logs"}}"[\s\S]*<span>Audit Logs<\/span>[\s\S]*href="{{appURL "\/admin\/dashboard\/usage"}}"[\s\S]*<span>Usage<\/span>[\s\S]*href="{{appURL "\/admin\/dashboard\/budgets"}}"[\s\S]*x-show="budgetManagementEnabled\(\)"[\s\S]*<span>Budgets<\/span>[\s\S]*href="{{appURL "\/admin\/dashboard\/auth-keys"}}"[\s\S]*<span>API Keys<\/span>[\s\S]*href="{{appURL "\/admin\/dashboard\/workflows"}}"[\s\S]*<span>Workflows<\/span>[\s\S]*href="{{appURL "\/admin\/dashboard\/guardrails"}}"[\s\S]*x-show="guardrailsPageVisible\(\)"[\s\S]*<span>Guardrails \(experimental\)<\/span>[\s\S]*href="{{appURL "\/admin\/dashboard\/settings"}}"[\s\S]*<span>Settings<\/span>/,
/href="{{appURL "\/admin\/dashboard\/overview"}}"[\s\S]*<span>Overview<\/span>[\s\S]*href="{{appURL "\/admin\/dashboard\/models"}}"[\s\S]*<span>Models<\/span>[\s\S]*href="{{appURL "\/admin\/dashboard\/audit-logs"}}"[\s\S]*<span>Audit Logs<\/span>[\s\S]*href="{{appURL "\/admin\/dashboard\/usage"}}"[\s\S]*<span>Usage<\/span>[\s\S]*href="{{appURL "\/admin\/dashboard\/budgets"}}"[\s\S]*x-show="budgetManagementEnabled\(\)"[\s\S]*<span>Budgets<\/span>[\s\S]*href="{{appURL "\/admin\/dashboard\/auth-keys"}}"[\s\S]*<span>API Keys<\/span>[\s\S]*href="{{appURL "\/admin\/dashboard\/workflows"}}"[\s\S]*<span>Workflows<\/span>[\s\S]*href="{{appURL "\/admin\/dashboard\/guardrails"}}"[\s\S]*x-show="guardrailsPageVisible\(\)"[\s\S]*<span>Guardrails \(experimental\)<\/span>[\s\S]*href="{{appURL "\/admin\/dashboard\/mcp-servers"}}"[\s\S]*x-show="mcpServersPageVisible\(\)"[\s\S]*<span>MCP Servers<\/span>[\s\S]*href="{{appURL "\/admin\/dashboard\/settings"}}"[\s\S]*<span>Settings<\/span>/,
);

const sidebarRule = readCSSRule(css, ".sidebar");
Expand Down
14 changes: 14 additions & 0 deletions internal/admin/dashboard/static/js/modules/mcp-servers.js
Original file line number Diff line number Diff line change
Expand Up @@ -292,6 +292,20 @@
},

async fetchMcpServersPage() {
// dashboardDataFetches starts feature-backed requests in
// parallel, so wait for the shared runtime-config request
// before deciding whether the MCP admin API is available.
if (typeof this.ensureWorkflowRuntimeConfig === 'function') {
await this.ensureWorkflowRuntimeConfig();
}
if (typeof this.mcpServersPageVisible === 'function' && !this.mcpServersPageVisible()) {
this.mcpServersAvailable = false;
this.mcpServers = [];
this.mcpServerError = '';
this.mcpServersLoading = false;
return;
}

Comment on lines 294 to +308

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win

Set the MCP loading state before awaiting runtime configuration.

mcpServersLoading remains false while ensureWorkflowRuntimeConfig() is pending, so a direct visit can render the empty state before the feature flag resolves. Move the existing loading assignment before the await and keep clearing it in the disabled and finally paths.

Proposed fix
 async fetchMcpServersPage() {
+    this.mcpServersLoading = true;
     if (typeof this.ensureWorkflowRuntimeConfig === 'function') {
         await this.ensureWorkflowRuntimeConfig();
     }
...
-    this.mcpServersLoading = true;
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
async fetchMcpServersPage() {
// dashboardDataFetches starts feature-backed requests in
// parallel, so wait for the shared runtime-config request
// before deciding whether the MCP admin API is available.
if (typeof this.ensureWorkflowRuntimeConfig === 'function') {
await this.ensureWorkflowRuntimeConfig();
}
if (typeof this.mcpServersPageVisible === 'function' && !this.mcpServersPageVisible()) {
this.mcpServersAvailable = false;
this.mcpServers = [];
this.mcpServerError = '';
this.mcpServersLoading = false;
return;
}
async fetchMcpServersPage() {
this.mcpServersLoading = true;
// dashboardDataFetches starts feature-backed requests in
// parallel, so wait for the shared runtime-config request
// before deciding whether the MCP admin API is available.
if (typeof this.ensureWorkflowRuntimeConfig === 'function') {
await this.ensureWorkflowRuntimeConfig();
}
if (typeof this.mcpServersPageVisible === 'function' && !this.mcpServersPageVisible()) {
this.mcpServersAvailable = false;
this.mcpServers = [];
this.mcpServerError = '';
this.mcpServersLoading = false;
return;
}
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@internal/admin/dashboard/static/js/modules/mcp-servers.js` around lines 294 -
308, Update fetchMcpServersPage so mcpServersLoading is set to true before
awaiting ensureWorkflowRuntimeConfig. Preserve the existing clearing behavior in
the disabled return path and the finally path, ensuring the loading state
remains active while runtime configuration resolves.

this.mcpServersLoading = true;
this.mcpServerError = '';
try {
Expand Down
28 changes: 28 additions & 0 deletions internal/admin/dashboard/static/js/modules/mcp-servers.test.cjs
Original file line number Diff line number Diff line change
Expand Up @@ -139,6 +139,34 @@ test('list normalization splits comma tools and newline user paths', () => {
assert.equal(JSON.stringify(module.normalizeMcpUserPaths('/\n /team/alpha \n\n')), JSON.stringify(['/', '/team/alpha']));
});

test('fetchMcpServersPage waits for runtime config and skips the disabled endpoint', async () => {
let configLoaded = false;
let fetchCalls = 0;
const module = createMcpServersModule({
fetch: async () => {
fetchCalls++;
return { status: 200, json: async () => [] };
}
});
Object.assign(module, {
ensureWorkflowRuntimeConfig: async () => {
configLoaded = true;
},
mcpServersPageVisible: () => {
assert.equal(configLoaded, true);
return false;
}
});

await module.fetchMcpServersPage();

assert.equal(fetchCalls, 0);
assert.equal(module.mcpServersAvailable, false);
assert.equal(module.mcpServers.length, 0);
assert.equal(module.mcpServersLoading, false);
assert.equal(module.mcpServerError, '');
});

test('fetchMcpServersPage marks the feature unavailable on 404 and 503', async () => {
for (const status of [404, 503]) {
const module = createMcpServersModule({
Expand Down
3 changes: 2 additions & 1 deletion internal/admin/dashboard/static/js/modules/workflows.js
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@
'REDIS_URL',
'SEMANTIC_CACHE_ENABLED',
'USAGE_PRICING_RECALCULATION_ENABLED',
'DASHBOARD_LIVE_LOGS_ENABLED'
'DASHBOARD_LIVE_LOGS_ENABLED',
'MCP_ENABLED'
];
},

Expand Down
2 changes: 1 addition & 1 deletion internal/admin/dashboard/templates/sidebar.html
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ <h1>GoModel</h1>
<i data-lucide="shield-check" class="nav-icon" aria-hidden="true"></i>
<span>Guardrails (experimental)</span>
</a>
<a href="{{appURL "/admin/dashboard/mcp-servers"}}" class="nav-item" :class="{ active: page === 'mcp-servers' }" title="MCP Servers" @click.prevent="navigate('mcp-servers')">
<a href="{{appURL "/admin/dashboard/mcp-servers"}}" class="nav-item" :class="{ active: page === 'mcp-servers' }" title="MCP Servers" x-show="mcpServersPageVisible()" @click.prevent="navigate('mcp-servers')">
<i data-lucide="plug" class="nav-icon" aria-hidden="true"></i>
<span>MCP Servers</span>
</a>
Expand Down
3 changes: 3 additions & 0 deletions internal/admin/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@ const (
DashboardConfigSemanticCacheEnabled = "SEMANTIC_CACHE_ENABLED"
DashboardConfigPricingRecalculation = "USAGE_PRICING_RECALCULATION_ENABLED"
DashboardConfigLiveLogsEnabled = "DASHBOARD_LIVE_LOGS_ENABLED"
DashboardConfigMCPEnabled = "MCP_ENABLED"
)

// statusClientClosedRequest is the de facto status used by proxies for client-aborted requests.
Expand All @@ -96,6 +97,7 @@ type DashboardConfigResponse struct {
SemanticCacheEnabled string `json:"SEMANTIC_CACHE_ENABLED,omitempty"`
PricingRecalculation string `json:"USAGE_PRICING_RECALCULATION_ENABLED,omitempty"`
LiveLogsEnabled string `json:"DASHBOARD_LIVE_LOGS_ENABLED,omitempty"`
MCPEnabled string `json:"MCP_ENABLED,omitempty"`
}

type providerStatusSummaryResponse struct {
Expand Down Expand Up @@ -352,6 +354,7 @@ func normalizeDashboardRuntimeConfig(values DashboardConfigResponse) DashboardCo
SemanticCacheEnabled: strings.TrimSpace(values.SemanticCacheEnabled),
PricingRecalculation: strings.TrimSpace(values.PricingRecalculation),
LiveLogsEnabled: strings.TrimSpace(values.LiveLogsEnabled),
MCPEnabled: strings.TrimSpace(values.MCPEnabled),
}
}

Expand Down
4 changes: 4 additions & 0 deletions internal/admin/handler_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -2263,6 +2263,7 @@ func TestDashboardConfig_ReturnsAllowlistedRuntimeFlags(t *testing.T) {
SemanticCacheEnabled: "off",
PricingRecalculation: "on",
LiveLogsEnabled: "on",
MCPEnabled: "off",
}))
c, rec := newHandlerContext("/admin/runtime/config")

Expand Down Expand Up @@ -2316,6 +2317,9 @@ func TestDashboardConfig_ReturnsAllowlistedRuntimeFlags(t *testing.T) {
if got := body.LiveLogsEnabled; got != "on" {
t.Fatalf("DASHBOARD_LIVE_LOGS_ENABLED = %q, want on", got)
}
if got := body.MCPEnabled; got != "off" {
t.Fatalf("MCP_ENABLED = %q, want off", got)
}
if rec.Body.String() == "" || strings.Contains(rec.Body.String(), "UNRELATED_FLAG") {
t.Fatal("UNRELATED_FLAG should not be exposed")
}
Expand Down
1 change: 1 addition & 0 deletions internal/app/app.go
Original file line number Diff line number Diff line change
Expand Up @@ -1344,6 +1344,7 @@ func dashboardRuntimeConfig(cfg *config.Config, usageEnabled, demoMode bool) adm
RedisURL: dashboardEnabledValue(simpleResponseCacheConfigured(cfg)),
SemanticCacheEnabled: dashboardEnabledValue(semanticResponseCacheConfigured(cfg)),
LiveLogsEnabled: dashboardEnabledValue(cfg != nil && cfg.Admin.LiveLogsEnabled),
MCPEnabled: dashboardEnabledValue(cfg != nil && cfg.MCP.Enabled),
}
}

Expand Down
15 changes: 15 additions & 0 deletions internal/app/app_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -501,6 +501,9 @@ func TestDashboardRuntimeConfig_ExposesFeatureAvailabilityFlags(t *testing.T) {
Admin: config.AdminConfig{
LiveLogsEnabled: true,
},
MCP: config.MCPConfig{
Enabled: true,
},
Cache: config.CacheConfig{
Response: config.ResponseCacheConfig{
Simple: &config.SimpleCacheConfig{
Expand Down Expand Up @@ -541,6 +544,9 @@ func TestDashboardRuntimeConfig_ExposesFeatureAvailabilityFlags(t *testing.T) {
if got := values.LiveLogsEnabled; got != "on" {
t.Fatalf("dashboardRuntimeConfig()[%q] = %q, want on", admin.DashboardConfigLiveLogsEnabled, got)
}
if got := values.MCPEnabled; got != "on" {
t.Fatalf("dashboardRuntimeConfig()[%q] = %q, want on", admin.DashboardConfigMCPEnabled, got)
}
}

func TestDashboardRuntimeConfig_ExposesIndefiniteLoggingRetention(t *testing.T) {
Expand All @@ -550,6 +556,15 @@ func TestDashboardRuntimeConfig_ExposesIndefiniteLoggingRetention(t *testing.T)
}
}

func TestDashboardRuntimeConfig_HidesMCPWhenDisabled(t *testing.T) {
values := dashboardRuntimeConfig(&config.Config{
MCP: config.MCPConfig{Enabled: false},
}, false, false)
if got := values.MCPEnabled; got != "off" {
t.Fatalf("dashboardRuntimeConfig()[%q] = %q, want off", admin.DashboardConfigMCPEnabled, got)
}
}

func TestDashboardRuntimeConfig_HidesCacheAnalyticsWhenUsageDisabled(t *testing.T) {
cfg := &config.Config{
Usage: config.UsageConfig{
Expand Down