Skip to content

Commit ff03db9

Browse files
authored
fix(049): classifyServerToolStatus must use runtime config authority (#477)
The merged #476 classifyServerToolStatus read config-denial only from the storage ServerConfig copy. Config-file disabled_tools on stdio servers live in the live runtime config and are not always mirrored to storage, so disabled_by_config classification + upstream_servers counts were wrong at runtime for that case (unit tests passed because the no-runtime harness uses the storage fallback path). Prefer p.mainServer.runtime.IsToolConfigDenied (same authority isToolCallable/blockedToolMessage use); fall back to storage IsToolAllowedByConfig only when no runtime is wired. Adds a runtime regression test pinning that ClassifyDisabledTool reads live config (the authority the server layer now delegates to). Related #468
1 parent 95f5fa4 commit ff03db9

2 files changed

Lines changed: 29 additions & 3 deletions

File tree

internal/runtime/tool_disabled_classify_test.go

Lines changed: 17 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,3 +79,20 @@ func BenchmarkClassifyDisabledTool(b *testing.B) {
7979
_ = rt.ClassifyDisabledTool("github", "delete_repo")
8080
}
8181
}
82+
83+
// Regression for the #476 follow-up: config-file disabled_tools live ONLY in
84+
// the runtime config, not always in the storage copy. ClassifyDisabledTool
85+
// must read the live config (r.Config()) so disabled_by_config is correct for
86+
// config-file stdio servers — the server-layer classifier delegates its
87+
// config-denied leg to this authority.
88+
func TestClassifyDisabledTool_ConfigFromLiveConfig(t *testing.T) {
89+
rt := setupConfigFilterRuntime(t, []*config.ServerConfig{
90+
{Name: "cfgsrv", Enabled: true, DisabledTools: []string{"locked"}},
91+
})
92+
// No storage approval record and no storage.SaveUpstreamServer call:
93+
// disabled_tools exists only in the live config here.
94+
assert.Equal(t, contracts.DisabledStatusByConfig,
95+
rt.ClassifyDisabledTool("cfgsrv", "locked"))
96+
assert.Equal(t, contracts.DisabledStatusUnknown,
97+
rt.ClassifyDisabledTool("cfgsrv", "allowed")) // callable → not a disable reason
98+
}

internal/server/mcp.go

Lines changed: 12 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -4665,8 +4665,11 @@ func (p *MCPProxyServer) serverToolCounts(serverName string, toolNames []string)
46654665
}
46664666

46674667
// classifyServerToolStatus returns "" when the tool is callable, otherwise the
4668-
// disable reason. Storage-sourced, runtime-independent, mirrors
4669-
// classifyDisabledTool's precedence so the two never drift.
4668+
// disable reason. Mirrors classifyDisabledTool's precedence so the two never
4669+
// drift. The config-denied leg prefers the live runtime signal (the same
4670+
// authority isToolCallable/blockedToolMessage use — config-file disabled_tools
4671+
// only lives in the live config, not always in the storage copy); it falls
4672+
// back to the storage ServerConfig when no runtime is wired (unit tests).
46704673
func (p *MCPProxyServer) classifyServerToolStatus(serverName, toolName string) contracts.DisabledToolStatus {
46714674
if strings.Contains(toolName, ":") {
46724675
if parts := strings.SplitN(toolName, ":", 2); len(parts) == 2 {
@@ -4683,7 +4686,13 @@ func (p *MCPProxyServer) classifyServerToolStatus(serverName, toolName string) c
46834686
if !sc.Enabled {
46844687
return contracts.DisabledStatusServerDisabled
46854688
}
4686-
if !sc.IsToolAllowedByConfig(toolName) {
4689+
configDenied := false
4690+
if p.mainServer != nil && p.mainServer.runtime != nil {
4691+
configDenied = p.mainServer.runtime.IsToolConfigDenied(serverName, toolName)
4692+
} else {
4693+
configDenied = !sc.IsToolAllowedByConfig(toolName)
4694+
}
4695+
if configDenied {
46874696
return contracts.DisabledStatusByConfig
46884697
}
46894698
if approval, aerr := p.storage.GetToolApproval(serverName, toolName); aerr == nil && approval != nil {

0 commit comments

Comments
 (0)