feat(prompts): polish for upstream prompt aggregation (F6/F7, F9, F14, F15)

cmd/generate-types/main.go

@@ -233,6 +233,7 @@ export interface PreflightResponse {
   user_logged_out?: boolean; // True if user explicitly logged out (prevents auto-reconnection)
   health?: HealthStatus; // Unified health status calculated by the backend
   trust_mode?: string; // Per-server approval trust mode (spec 086): 'auto' | 'scan' | 'manual'; raw configured value, absent when unset (effective default: manual)
+  expose_prompts?: boolean; // F9 per-server prompt-aggregation override; absent = inherit default aggregation, false = exclude this server's prompts
   security_scan?: SecurityScanSummary; // Latest scan summary (spec 086); ABSENT when no scan has ever run
   // Spec 093 (#955) per-server concurrency overrides. Tri-state: absent =
   // inherit server_concurrency_defaults, 0 = disabled for this server,

frontend/src/types/contracts.ts

@@ -175,6 +175,7 @@ export interface Server {
   user_logged_out?: boolean; // True if user explicitly logged out (prevents auto-reconnection)
   health?: HealthStatus; // Unified health status calculated by the backend
   trust_mode?: string; // Per-server approval trust mode (spec 086): 'auto' | 'scan' | 'manual'; raw configured value, absent when unset (effective default: manual)
+  expose_prompts?: boolean; // F9 per-server prompt-aggregation override; absent = inherit default aggregation, false = exclude this server's prompts
   security_scan?: SecurityScanSummary; // Latest scan summary (spec 086); ABSENT when no scan has ever run
   // Spec 093 (#955) per-server concurrency overrides. Tri-state: absent =
   // inherit server_concurrency_defaults, 0 = disabled for this server,

internal/config/config.go

@@ -2269,6 +2269,20 @@ func (c *Config) ValidateDetailed() []ValidationError {
 				Field:   fieldPrefix + ".name",
 				Message: fmt.Sprintf("duplicate server name: %s", server.Name),
 			})
+		} else if strings.Contains(server.Name, ":") {
+			// F6: ':' is the qualified-name separator for prompt aggregation
+			// ("server:prompt", manager_prompts.go) and tool routing (CallTool's
+			// SplitN ":"). A name containing it makes the first-separator split
+			// route to the wrong server, so such a server's prompts/tools can never
+			// be reached correctly. Reject it — no working config uses ':' since it
+			// has never routed. ('__', the direct-mode display separator, is NOT
+			// hard-rejected here for back-compat: it works in retrieve_tools mode
+			// and any residual display collision is logged + handled deterministically
+			// in buildAggregatedServerPrompts / buildDirectModeTools.)
+			errors = append(errors, ValidationError{
+				Field:   fieldPrefix + ".name",
+				Message: fmt.Sprintf("server name %q must not contain ':' (reserved as the server:tool / server:prompt routing separator)", server.Name),
+			})
 		} else {
 			serverNames[server.Name] = true
 		}

internal/config/server_name_validation_test.go

@@ -0,0 +1,49 @@
+package config
+
+import (
+	"strings"
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+	"github.com/stretchr/testify/require"
+)
+
+// TestValidateDetailed_ServerNameColon covers Finding F6: a server name
+// containing ':' breaks the "server:prompt" / "server:tool" routing split, so
+// it must be rejected at config load rather than silently misrouting.
+func TestValidateDetailed_ServerNameColon(t *testing.T) {
+	newCfg := func(name string) *Config {
+		cfg := DefaultConfig()
+		cfg.Servers = []*ServerConfig{{
+			Name:     name,
+			URL:      "https://example.com/mcp",
+			Protocol: "streamable-http",
+		}}
+		return cfg
+	}
+
+	t.Run("colon in name is rejected", func(t *testing.T) {
+		errs := newCfg("team:gh").ValidateDetailed()
+		var found *ValidationError
+		for i := range errs {
+			if strings.HasSuffix(errs[i].Field, ".name") {
+				found = &errs[i]
+				break
+			}
+		}
+		require.NotNil(t, found, "a ':' server name must produce a validation error, got %+v", errs)
+		assert.Contains(t, found.Message, "team:gh")
+		assert.Contains(t, found.Message, "':'")
+	})
+
+	// '__' is intentionally NOT hard-rejected (back-compat: it works in
+	// retrieve_tools mode and any residual display collision is logged and
+	// handled deterministically at aggregation time).
+	for _, name := range []string{"github", "db_server", "my__server", "server-a"} {
+		t.Run("valid "+name, func(t *testing.T) {
+			for _, e := range newCfg(name).ValidateDetailed() {
+				assert.NotContains(t, e.Field, ".name", "valid server name %q must not error: %+v", name, e)
+			}
+		})
+	}
+}

internal/contracts/converters.go

@@ -42,6 +42,9 @@ func ConvertServerConfig(cfg *config.ServerConfig, status string, connected bool
 		// MCP-3322: surface the per-server init_timeout override so callers can
 		// read back a configured handshake deadline.
 		InitTimeout: cfg.InitTimeout,
+		// F9: surface the per-server prompt-aggregation override so a caller that
+		// PATCHed it can read it back.
+		ExposePrompts: cfg.ExposePrompts,
 		// Spec 093: surface the per-server concurrency overrides (tri-state) so a
 		// caller that PATCHed a limit can read it back.
 		MaxConcurrentRequests: cfg.MaxConcurrentRequests,
@@ -217,6 +220,12 @@ func ConvertGenericServersToTyped(genericServers []map[string]interface{}) []Ser
 			v := autoApprove
 			server.AutoApproveToolChanges = &v
 		}
+		// F9: prompt-aggregation override is tri-state — only set the pointer when
+		// the key is present so an unset override stays nil.
+		if exposePrompts, ok := generic["expose_prompts"].(bool); ok {
+			v := exposePrompts
+			server.ExposePrompts = &v
+		}
 		// Spec 086: per-server trust tier round-trips as a plain string.
 		if trustMode, ok := generic["trust_mode"].(string); ok {
 			server.TrustMode = trustMode

internal/contracts/types.go

@@ -72,8 +72,13 @@ type Server struct {
 	// a duration string (e.g. "120s"); nil/omitted means "inherit the global
 	// default". Surfaced on the GET path so clients can read back a configured
 	// override; PATCH/POST accept it via AddServerRequest.
-	InitTimeout  *config.Duration     `json:"init_timeout,omitempty" swaggertype:"string"`
-	SecurityScan *SecurityScanSummary `json:"security_scan,omitempty"` // Latest security scan results summary
+	InitTimeout *config.Duration `json:"init_timeout,omitempty" swaggertype:"string"`
+	// ExposePrompts mirrors config.ServerConfig.ExposePrompts (F9): the per-server
+	// prompt-aggregation override. Tri-state *bool — nil/omitted means "inherit
+	// default aggregation". Surfaced on GET so a caller that PATCHed the override
+	// can read it back; PATCH/POST accept it via AddServerRequest.
+	ExposePrompts *bool                `json:"expose_prompts,omitempty"`
+	SecurityScan  *SecurityScanSummary `json:"security_scan,omitempty"` // Latest security scan results summary
 	// Spec 044 — structured diagnostic error and stable error code. Both
 	// are populated when the server is in a failed state and the error
 	// has been classified by internal/diagnostics. Healthy servers omit

internal/httpapi/patch_server_test.go

@@ -960,3 +960,60 @@ func TestHandleGetServers_ExposesConcurrencyOverrides(t *testing.T) {
 	assert.Equal(t, "45s", resp.Data.Servers[0].QueueTimeout,
 		"queue_timeout must appear in the GET payload as a duration string")
 }
+
+// TestHandlePatchServer_ExposePrompts verifies F9: the per-server expose_prompts
+// override is reachable via PATCH. An explicit false must be mapped into
+// ServerConfig.ExposePrompts (previously the request had no field, so PATCH
+// {"expose_prompts":false} returned 400 "No fields to update"), and omitting it
+// must preserve the existing pointer.
+func TestHandlePatchServer_ExposePrompts(t *testing.T) {
+	logger := zap.NewNop().Sugar()
+
+	t.Run("explicit false is applied and does not 400", func(t *testing.T) {
+		mockCtrl := &mockPatchServerController{
+			apiKey: "test-key",
+			existingServer: &config.ServerConfig{
+				Name: "github", Protocol: "stdio", Enabled: true,
+			},
+		}
+		srv := NewServer(mockCtrl, logger, nil)
+
+		body, _ := json.Marshal(map[string]any{"expose_prompts": false})
+		req := httptest.NewRequest(http.MethodPatch, "/api/v1/servers/github", bytes.NewReader(body))
+		req.Header.Set("Content-Type", "application/json")
+		req.Header.Set("X-API-Key", "test-key")
+		w := httptest.NewRecorder()
+
+		srv.ServeHTTP(w, req)
+		require.Equal(t, http.StatusOK, w.Code, "body=%s", w.Body.String())
+		require.NotNil(t, mockCtrl.capturedUpdates)
+		require.NotNil(t, mockCtrl.capturedUpdates.ExposePrompts,
+			"expose_prompts from the PATCH body must be mapped into ServerConfig")
+		assert.False(t, *mockCtrl.capturedUpdates.ExposePrompts)
+	})
+
+	t.Run("omitted expose_prompts preserves existing", func(t *testing.T) {
+		existing := true
+		mockCtrl := &mockPatchServerController{
+			apiKey: "test-key",
+			existingServer: &config.ServerConfig{
+				Name: "github", Protocol: "stdio", Enabled: true,
+				ExposePrompts: &existing,
+			},
+		}
+		srv := NewServer(mockCtrl, logger, nil)
+
+		body, _ := json.Marshal(map[string]any{"args": []string{"x"}})
+		req := httptest.NewRequest(http.MethodPatch, "/api/v1/servers/github", bytes.NewReader(body))
+		req.Header.Set("Content-Type", "application/json")
+		req.Header.Set("X-API-Key", "test-key")
+		w := httptest.NewRecorder()
+
+		srv.ServeHTTP(w, req)
+		require.Equal(t, http.StatusOK, w.Code, "body=%s", w.Body.String())
+		require.NotNil(t, mockCtrl.capturedUpdates)
+		require.NotNil(t, mockCtrl.capturedUpdates.ExposePrompts,
+			"omitted expose_prompts must preserve the existing pointer")
+		assert.True(t, *mockCtrl.capturedUpdates.ExposePrompts)
+	})
+}

internal/httpapi/server.go

@@ -1585,6 +1585,12 @@ type AddServerRequest struct {
 	// semantics — do NOT collapse to a plain bool, or an omitted field would
 	// silently reset a previously-set value.
 	AutoApproveToolChanges *bool `json:"auto_approve_tool_changes,omitempty"`
+	// ExposePrompts is the per-server override for prompt aggregation (F9):
+	// whether this server's advertised MCP prompts are merged into mcpproxy's
+	// prompts/list. Tri-state *bool mirroring config.ServerConfig.ExposePrompts —
+	// a nil pointer means "leave unchanged" on PATCH (and "inherit the default
+	// aggregate behavior" on create); a present value (including false) is applied.
+	ExposePrompts *bool `json:"expose_prompts,omitempty"`
 	// TrustMode is the per-server trust tier (spec 086): "auto", "scan", or
 	// "manual". Empty means "leave unchanged" on PATCH (and inherit the migrated
 	// default on create). A non-empty value is applied to ServerConfig.TrustMode
@@ -1769,6 +1775,12 @@ func (s *Server) handleAddServer(w http.ResponseWriter, r *http.Request) {
 	if req.AutoApproveToolChanges != nil {
 		serverConfig.AutoApproveToolChanges = req.AutoApproveToolChanges
 	}
+	// F9: carry the per-server prompt-aggregation override through on create.
+	// Pointer-assign (config field is *bool) so an omitted field stays nil =
+	// "inherit default aggregation".
+	if req.ExposePrompts != nil {
+		serverConfig.ExposePrompts = req.ExposePrompts
+	}
 	// Spec 086: carry the per-server trust_mode through on create. Empty means
 	// "not specified" — leave it for the loader's legacy-flag migration to
 	// populate; a present value wins.
@@ -2032,6 +2044,15 @@ func (s *Server) handlePatchServer(w http.ResponseWriter, r *http.Request) {
 	} else if existingSrv != nil {
 		updates.AutoApproveToolChanges = existingSrv.AutoApproveToolChanges
 	}
+	// F9: expose_prompts is a tri-state *bool — preserve the EXISTING POINTER
+	// (which may be nil = "never set") when the request omits the field, so a
+	// bare PATCH of an unrelated field does not wipe a configured override.
+	if req.ExposePrompts != nil {
+		updates.ExposePrompts = req.ExposePrompts
+		hasUpdates = true
+	} else if existingSrv != nil {
+		updates.ExposePrompts = existingSrv.ExposePrompts
+	}
 	// Spec 086: trust_mode is a plain string — empty means "leave unchanged", so
 	// preserve the existing value when the request omits it (a bare PATCH of an
 	// unrelated field must not reset the trust tier).

internal/runtime/runtime.go

@@ -2228,6 +2228,14 @@ func (r *Runtime) GetAllServers() ([]map[string]interface{}, error) {
 			serverMap["auto_approve_tool_changes"] = *serverStatus.Config.AutoApproveToolChanges
 		}
 
+		// F9: surface the per-server expose_prompts override so the REST GET
+		// payload can read back a configured value. Tri-state *bool — only emit
+		// the key when set so the projection stays nil for servers that never
+		// configured it.
+		if serverStatus.Config != nil && serverStatus.Config.ExposePrompts != nil {
+			serverMap["expose_prompts"] = *serverStatus.Config.ExposePrompts
+		}
+
 		// Spec 086: surface the per-server trust tier so the REST GET payload
 		// (and SSE servers.changed embed) can read back the persisted mode, in
 		// parity with its deprecated predecessor auto_approve_tool_changes.
@@ -2419,6 +2427,12 @@ func (r *Runtime) getAllServersLegacy() ([]map[string]interface{}, error) {
 			serverInfo["auto_approve_tool_changes"] = *srv.AutoApproveToolChanges
 		}
 
+		// F9: per-server expose_prompts override in parity with the StateView
+		// path. Tri-state *bool — only emit when set.
+		if srv.ExposePrompts != nil {
+			serverInfo["expose_prompts"] = *srv.ExposePrompts
+		}
+
 		// Spec 086: per-server trust tier in parity with the StateView path.
 		// Raw configured value; omitted when never configured.
 		if srv.TrustMode != "" {

internal/server/mcp_routing.go

@@ -705,19 +705,46 @@ func buildAggregatedServerPrompts(
 	builtins []mcpserver.ServerPrompt,
 	upstreamPrompts []mcp.Prompt,
 	getPrompt func(ctx context.Context, name string, args map[string]string) (*mcp.GetPromptResult, error),
+	logger *zap.Logger,
 ) []mcpserver.ServerPrompt {
 	all := make([]mcpserver.ServerPrompt, 0, len(builtins)+len(upstreamPrompts))
 	all = append(all, builtins...)
 
+	// F7: two distinct (server,prompt) pairs can flatten to the same "__" display
+	// name (server "a__b"+prompt "c" and "a"+prompt "b__c" both -> "a__b__c").
+	// mcp-go's SetPrompts is last-writer-wins by map order, so without this the
+	// loser is dropped silently. Config validation rejects ':' in names but keeps
+	// '__' for back-compat, so a residual collision can still occur — keep a
+	// deterministic first-writer-wins guard here so it is LOGGED, never silent.
+	// Built-in names are seeded into the seen-set so an upstream cannot shadow
+	// "setup-new-mcp-server"/"troubleshoot-mcp-server".
+	seen := make(map[string]struct{}, len(all)+len(upstreamPrompts))
+	for i := range all {
+		seen[all[i].Prompt.Name] = struct{}{}
+	}
+
 	for _, qualified := range upstreamPrompts {
 		serverName, promptName, ok := strings.Cut(qualified.Name, ":")
 		if !ok {
 			continue
 		}
 
+		displayName := FormatDirectPromptName(serverName, promptName)
+		if _, dup := seen[displayName]; dup {
+			if logger != nil {
+				logger.Warn("dropping upstream prompt: display-name collision (kept first)",
+					zap.String("server", serverName),
+					zap.String("prompt", promptName),
+					zap.String("display_name", displayName),
+					zap.String("qualified_name", qualified.Name))
+			}
+			continue
+		}
+		seen[displayName] = struct{}{}
+
 		qualifiedName := qualified.Name
 		display := qualified
-		display.Name = FormatDirectPromptName(serverName, promptName)
+		display.Name = displayName
 
 		all = append(all, mcpserver.ServerPrompt{
 			Prompt: display,
@@ -768,14 +795,14 @@ func (p *MCPProxyServer) RefreshPrompts() {
 		// scanner before they are ever registered (parity with tool-description
 		// poisoning detection).
 		upstreamPrompts = p.scanAggregatedPrompts(upstreamPrompts)
-		all = buildAggregatedServerPrompts(builtins, upstreamPrompts, p.getPromptAggregated)
+		all = buildAggregatedServerPrompts(builtins, upstreamPrompts, p.getPromptAggregated, p.logger)
 		p.logger.Info("refreshed prompts",
 			zap.Int("upstream_prompt_count", len(upstreamPrompts)),
 			zap.Int("total_prompt_count", len(all)))
 	} else {
 		// nil upstreamPrompts: the aggregation loop never runs, so the nil
 		// getPrompt is never invoked.
-		all = buildAggregatedServerPrompts(builtins, nil, nil)
+		all = buildAggregatedServerPrompts(builtins, nil, nil, p.logger)
 		p.logger.Debug("refreshed prompts (upstream aggregation disabled, built-ins only)",
 			zap.Int("total_prompt_count", len(all)))
 	}

internal/server/mcp_routing_test.go

@@ -12,6 +12,7 @@ import (
 	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"go.uber.org/zap"
+	"go.uber.org/zap/zaptest/observer"
 
 	"github.com/smart-mcp-proxy/mcpproxy-go/internal/auth"
 	"github.com/smart-mcp-proxy/mcpproxy-go/internal/config"
@@ -179,7 +180,7 @@ func TestBuildAggregatedServerPrompts(t *testing.T) {
 		return &mcp.GetPromptResult{Description: "from upstream"}, nil
 	}
 
-	all := buildAggregatedServerPrompts([]mcpserver.ServerPrompt{builtin}, upstreamPrompts, fakeGetPrompt)
+	all := buildAggregatedServerPrompts([]mcpserver.ServerPrompt{builtin}, upstreamPrompts, fakeGetPrompt, zap.NewNop())
 
 	require.Len(t, all, 2)
 	assert.Equal(t, "setup-new-mcp-server", all[0].Prompt.Name)
@@ -199,7 +200,7 @@ func TestBuildAggregatedServerPrompts(t *testing.T) {
 
 func TestBuildAggregatedServerPrompts_SkipsMalformedNames(t *testing.T) {
 	upstreamPrompts := []mcp.Prompt{{Name: "no-colon-here"}}
-	all := buildAggregatedServerPrompts(nil, upstreamPrompts, nil)
+	all := buildAggregatedServerPrompts(nil, upstreamPrompts, nil, zap.NewNop())
 	assert.Empty(t, all)
 }
 
@@ -1114,3 +1115,31 @@ func TestRefreshPrompts_PopulatesRoutingModeServers(t *testing.T) {
 		assert.Contains(t, prompts, "server-a__greeting", "mode %s: aggregated upstream prompt must be registered", mode)
 	}
 }
+
+// TestBuildAggregatedServerPrompts_CollisionKeepsFirst covers Finding F7: two
+// distinct (server,prompt) pairs that flatten to the same "server__prompt"
+// display name must be resolved deterministically (first-writer-wins), not
+// silently overwritten, and the drop must be logged.
+func TestBuildAggregatedServerPrompts_CollisionKeepsFirst(t *testing.T) {
+	core, logs := observer.New(zap.WarnLevel)
+	logger := zap.New(core)
+
+	// "gh" + "issue__create" and "gh__issue" + "create" both flatten to
+	// "gh__issue__create".
+	upstreamPrompts := []mcp.Prompt{
+		{Name: "gh:issue__create", Description: "first"},
+		{Name: "gh__issue:create", Description: "second (collides)"},
+	}
+	fakeGetPrompt := func(_ context.Context, _ string, _ map[string]string) (*mcp.GetPromptResult, error) {
+		return &mcp.GetPromptResult{}, nil
+	}
+
+	all := buildAggregatedServerPrompts(nil, upstreamPrompts, fakeGetPrompt, logger)
+
+	names := make([]string, len(all))
+	for i, p := range all {
+		names[i] = p.Prompt.Name
+	}
+	assert.Equal(t, []string{"gh__issue__create"}, names, "colliding display name must appear once (first kept)")
+	require.Equal(t, 1, logs.FilterMessage("dropping upstream prompt: display-name collision (kept first)").Len())
+}