diff --git a/.gitignore b/.gitignore index 85435cf052..5b7250cff2 100644 --- a/.gitignore +++ b/.gitignore @@ -1,9 +1,19 @@ # Test artifacts tests/integration/testdata/ -# Binary -openclaw-go -pkg-helper +# Binary (anchored to repo root so we don't accidentally ignore +# the cmd/pkg-helper/ source directory, which breaks Docker builds +# on uploaders that pattern-match .gitignore against the whole tree +# — notably `railway up`). +/openclaw-go +/pkg-helper +/goclaw.exe +/goclaw-local.exe + +# Ad-hoc debug probes live here — never commit (they import internal/ +# packages from outside cmd/ and only exist for one-shot diagnostics). +/tmp-reset-bot/ +/tmp-probe*/ # IDE .idea/ diff --git a/.railwayignore b/.railwayignore new file mode 100644 index 0000000000..695eda14d5 --- /dev/null +++ b/.railwayignore @@ -0,0 +1,18 @@ +.git +.github +.vscode +.idea +.claude +ui/desktop +ui/simple-saas +ui/web/node_modules +ui/web/dist +**/node_modules +plans +skills-store +docs +tests +tmp +tmp-* +*.exe +._* diff --git a/go.mod b/go.mod index 254e041508..53672c33f1 100644 --- a/go.mod +++ b/go.mod @@ -33,7 +33,7 @@ require ( github.com/slack-go/slack v0.19.0 github.com/spf13/cobra v1.10.2 github.com/titanous/json5 v1.0.0 - github.com/wailsapp/wails/v2 v2.11.0 + github.com/wailsapp/wails/v2 v2.12.0 github.com/zalando/go-keyring v0.2.8 go.mau.fi/whatsmeow v0.0.0-20260327181659-02ec817e7cf4 go.opentelemetry.io/otel v1.40.0 @@ -51,6 +51,7 @@ require ( require ( cel.dev/expr v0.25.1 // indirect filippo.io/edwards25519 v1.1.0 // indirect + git.sr.ht/~jackmordaunt/go-toast/v2 v2.0.3 // indirect github.com/akutz/memconn v0.1.0 // indirect github.com/alexbrainman/sspi v0.0.0-20231016080023-1a75b4708caa // indirect github.com/antlr4-go/antlr/v4 v4.13.1 // indirect diff --git a/go.sum b/go.sum index faea3b887b..2ffffa6629 100644 --- a/go.sum +++ b/go.sum @@ -6,6 +6,8 @@ filippo.io/edwards25519 v1.1.0 h1:FNf4tywRC1HmFuKW5xopWpigGjJKiJSV0Cqo0cJWDaA= filippo.io/edwards25519 v1.1.0/go.mod h1:BxyFTGdWcka3PhytdK4V28tE5sGfRvvvRV7EaN4VDT4= filippo.io/mkcert v1.4.4 h1:8eVbbwfVlaqUM7OwuftKc2nuYOoTDQWqsoXmzoXZdbc= filippo.io/mkcert v1.4.4/go.mod h1:VyvOchVuAye3BoUsPUOOofKygVwLV2KQMVFJNRq+1dA= +git.sr.ht/~jackmordaunt/go-toast/v2 v2.0.3 h1:N3IGoHHp9pb6mj1cbXbuaSXV/UMKwmbKLf53nQmtqMA= +git.sr.ht/~jackmordaunt/go-toast/v2 v2.0.3/go.mod h1:QtOLZGz8olr4qH2vWK0QH0w0O4T9fEIjMuWpKUsH7nc= github.com/Azure/go-ansiterm v0.0.0-20250102033503-faa5f7b0171c h1:udKWzYgxTojEKWjV8V+WSxDXJ4NFATAsZjh8iIbsQIg= github.com/Azure/go-ansiterm v0.0.0-20250102033503-faa5f7b0171c/go.mod h1:xomTg63KZ2rFqZQzSB4Vz2SUXa1BpHTVz9L5PTmPC4E= github.com/BurntSushi/toml v1.5.0 h1:W5quZX/G/csjUnuI8SUYlsHs9M38FC7znL0lIO+DvMg= @@ -501,8 +503,8 @@ github.com/wailsapp/go-webview2 v1.0.22 h1:YT61F5lj+GGaat5OB96Aa3b4QA+mybD0Ggq6N github.com/wailsapp/go-webview2 v1.0.22/go.mod h1:qJmWAmAmaniuKGZPWwne+uor3AHMB5PFhqiK0Bbj8kc= github.com/wailsapp/mimetype v1.4.1 h1:pQN9ycO7uo4vsUUuPeHEYoUkLVkaRntMnHJxVwYhwHs= github.com/wailsapp/mimetype v1.4.1/go.mod h1:9aV5k31bBOv5z6u+QP8TltzvNGJPmNJD4XlAL3U+j3o= -github.com/wailsapp/wails/v2 v2.11.0 h1:seLacV8pqupq32IjS4Y7V8ucab0WZwtK6VvUVxSBtqQ= -github.com/wailsapp/wails/v2 v2.11.0/go.mod h1:jrf0ZaM6+GBc1wRmXsM8cIvzlg0karYin3erahI4+0k= +github.com/wailsapp/wails/v2 v2.12.0 h1:BHO/kLNWFHYjCzucxbzAYZWUjub1Tvb4cSguQozHn5c= +github.com/wailsapp/wails/v2 v2.12.0/go.mod h1:mo1bzK1DEJrobt7YrBjgxvb5Sihb1mhAY09hppbibQg= github.com/wk8/go-ordered-map/v2 v2.1.8 h1:5h/BUHu93oj4gIdvHHHGsScSTMijfx5PeYkE/fJgbpc= github.com/wk8/go-ordered-map/v2 v2.1.8/go.mod h1:5nJHM5DyteebpVlHnWMV0rPz6Zp7+xBAnxjb1X5vnTw= github.com/x448/float16 v0.8.4 h1:qLwI1I70+NjRFUR3zs1JPUCgaCXSh3SW62uAKT1mSBM= @@ -647,8 +649,8 @@ gvisor.dev/gvisor v0.0.0-20250205023644-9414b50a5633 h1:2gap+Kh/3F47cO6hAu3idFvs gvisor.dev/gvisor v0.0.0-20250205023644-9414b50a5633/go.mod h1:5DMfjtclAbTIjbXqO1qCe2K5GKKxWz2JHvCChuTcJEM= honnef.co/go/tools v0.7.0-0.dev.0.20251022135355-8273271481d0 h1:5SXjd4ET5dYijLaf0O3aOenC0Z4ZafIWSpjUzsQaNho= honnef.co/go/tools v0.7.0-0.dev.0.20251022135355-8273271481d0/go.mod h1:EPDDhEZqVHhWuPI5zPAsjU0U7v9xNIWjoOVyZ5ZcniQ= -howett.net/plist v1.0.0 h1:7CrbWYbPPO/PyNy38b2EB/+gYbjCe2DXBxgtOOZbSQM= -howett.net/plist v1.0.0/go.mod h1:lqaXoTrLY4hg8tnEzNru53gicrbv7rrk+2xJA/7hw9g= +howett.net/plist v1.0.2-0.20250314012144-ee69052608d9 h1:eeH1AIcPvSc0Z25ThsYF+Xoqbn0CI/YnXVYoTLFdGQw= +howett.net/plist v1.0.2-0.20250314012144-ee69052608d9/go.mod h1:fyFX5Hj5tP1Mpk8obqA9MZgXT416Q5711SDT7dQLTLk= modernc.org/cc/v4 v4.27.1 h1:9W30zRlYrefrDV2JE2O8VDtJ1yPGownxciz5rrbQZis= modernc.org/cc/v4 v4.27.1/go.mod h1:uVtb5OGqUKpoLWhqwNQo/8LwvoiEBLvZXIQ/SmO6mL0= modernc.org/ccgo/v4 v4.32.0 h1:hjG66bI/kqIPX1b2yT6fr/jt+QedtP2fqojG2VrFuVw= diff --git a/internal/agent/loop_mcp_user.go b/internal/agent/loop_mcp_user.go index 830307852c..5512b8c97a 100644 --- a/internal/agent/loop_mcp_user.go +++ b/internal/agent/loop_mcp_user.go @@ -88,8 +88,10 @@ func (l *Loop) getUserMCPTools(ctx context.Context, userID string) []tools.Tool // Create BridgeTools pointing to user's connection and register in the // shared tool registry so ExecuteWithContext can resolve them by name. reg, _ := l.tools.(*tools.Registry) + hints := mcpbridge.ParseToolHints(srv.Settings) for _, mcpTool := range entry.MCPTools() { - bt := mcpbridge.NewBridgeTool(srv.Name, mcpTool, entry.ClientPtr(), srv.ToolPrefix, srv.TimeoutSec, entry.Connected(), srv.ID, l.mcpGrantChecker) + bt := mcpbridge.NewBridgeTool(srv.Name, mcpTool, entry.ClientPtr(), srv.ToolPrefix, srv.TimeoutSec, entry.Connected(), srv.ID, l.mcpGrantChecker). + WithHints(hints.Global, hints.HintFor(mcpTool.Name)) // Register in registry so ExecuteWithContext can find them. // Skip if already registered (another user loaded this server with same tool names). if reg != nil { diff --git a/internal/gateway/methods/channel_instances.go b/internal/gateway/methods/channel_instances.go index 3f06f2c092..d8d8bf83ea 100644 --- a/internal/gateway/methods/channel_instances.go +++ b/internal/gateway/methods/channel_instances.go @@ -277,9 +277,15 @@ func maskInstance(inst store.ChannelInstanceData) map[string]any { } // isValidChannelType checks if the channel type is supported. +// +// Keep this list in sync with the HTTP twin in internal/http/channel_instances.go +// and with CHANNEL_TYPES in ui/web/src/constants/channels.ts. When the two +// backend switches drift (as happened with facebook/pancake/bitrix24), the +// WS-driven UI rejects channels the HTTP API accepts, and the dropdown offers +// channels neither API accepts. func isValidChannelType(ct string) bool { switch ct { - case "telegram", "discord", "slack", "whatsapp", "zalo_oa", "zalo_personal", "feishu": + case "telegram", "discord", "slack", "whatsapp", "zalo_oa", "zalo_personal", "feishu", "facebook", "pancake", "bitrix24": return true } return false diff --git a/internal/gateway/server.go b/internal/gateway/server.go index cbfb79fcd2..2fd85ec0a8 100644 --- a/internal/gateway/server.go +++ b/internal/gateway/server.go @@ -198,9 +198,17 @@ func (s *Server) BuildMux() *http.ServeMux { } // Embedded web UI (built with -tags embedui). Catch-all after all API routes. + // When the build does NOT include the embedui tag, webui.Handler() returns nil + // and there's no handler for "/" — http.ServeMux would then return an opaque + // 404 for the root URL, confusing operators who open the deployed URL in a + // browser to check the service. Install a minimal JSON index handler in that + // case so the root responds with something useful (and any unmatched path + // still returns 404, just with a JSON body). if h := webui.Handler(); h != nil { mux.Handle("/", h) slog.Info("serving embedded web UI") + } else { + mux.HandleFunc("/", s.handleIndex) } s.mux = mux @@ -372,6 +380,24 @@ func (s *Server) handleHealth(w http.ResponseWriter, r *http.Request) { fmt.Fprintf(w, `{"status":"ok","protocol":%d}`, protocol.ProtocolVersion) } +// handleIndex is the fallback "/" handler when no embedded web UI is present. +// It returns a small JSON service-info document for exact-match "/" requests +// and a JSON 404 for everything else — http.ServeMux routes "/" as a +// catch-all, so unrelated paths fall through here too. +func (s *Server) handleIndex(w http.ResponseWriter, r *http.Request) { + w.Header().Set("Content-Type", "application/json") + if r.URL.Path != "/" { + w.WriteHeader(http.StatusNotFound) + _, _ = w.Write([]byte(`{"error":"not found"}`)) + return + } + w.WriteHeader(http.StatusOK) + fmt.Fprintf(w, + `{"service":"goclaw","status":"ok","protocol":%d,`+ + `"endpoints":["/health","/v1/chat/completions","/v1/responses","/v1/tools/invoke","/ws"]}`, + protocol.ProtocolVersion) +} + // clientIP extracts the real client IP from the request, checking proxy headers first. func clientIP(r *http.Request) string { if ip := r.Header.Get("X-Real-IP"); ip != "" { diff --git a/internal/http/channel_instances.go b/internal/http/channel_instances.go index 180f87c545..08707e7db8 100644 --- a/internal/http/channel_instances.go +++ b/internal/http/channel_instances.go @@ -554,9 +554,13 @@ func (h *ChannelInstancesHandler) handleResolveContacts(w http.ResponseWriter, r } // isValidChannelType checks if the channel type is supported. +// +// Keep this list in sync with the WS twin in +// internal/gateway/methods/channel_instances.go and with CHANNEL_TYPES in +// ui/web/src/constants/channels.ts. func isValidChannelType(ct string) bool { switch ct { - case "telegram", "discord", "slack", "whatsapp", "zalo_oa", "zalo_personal", "feishu", "facebook", "pancake": + case "telegram", "discord", "slack", "whatsapp", "zalo_oa", "zalo_personal", "feishu", "facebook", "pancake", "bitrix24": return true } return false diff --git a/internal/mcp/bridge_tool.go b/internal/mcp/bridge_tool.go index 9bcaf338a3..773dc9312e 100644 --- a/internal/mcp/bridge_tool.go +++ b/internal/mcp/bridge_tool.go @@ -20,17 +20,18 @@ import ( // The client pointer is loaded atomically from clientPtr to support // safe reconnection without data races. type BridgeTool struct { - serverName string - serverID uuid.UUID // MCP server ID (for grant recheck) - toolName string // original MCP tool name - registeredName string // may include prefix: "{prefix}__{toolName}" - description string - inputSchema map[string]any // JSON Schema for parameters - requiredSet map[string]bool - clientPtr *atomic.Pointer[mcpclient.Client] // shared with serverState for atomic swap on reconnect - timeoutSec int - connected *atomic.Bool - grantChecker GrantChecker // for runtime grant recheck (nil = skip check) + serverName string + serverID uuid.UUID // MCP server ID (for grant recheck) + toolName string // original MCP tool name + registeredName string // may include prefix: "{prefix}__{toolName}" + description string + descriptionSuffix string // admin-authored hints appended to description (see WithHints) + inputSchema map[string]any // JSON Schema for parameters + requiredSet map[string]bool + clientPtr *atomic.Pointer[mcpclient.Client] // shared with serverState for atomic swap on reconnect + timeoutSec int + connected *atomic.Bool + grantChecker GrantChecker // for runtime grant recheck (nil = skip check) } // NewBridgeTool creates a BridgeTool from an MCP Tool definition. @@ -92,10 +93,42 @@ func ensureMCPPrefix(prefix, serverName string) string { return prefix } -func (t *BridgeTool) Name() string { return t.registeredName } -func (t *BridgeTool) Description() string { return t.description } +func (t *BridgeTool) Name() string { return t.registeredName } +func (t *BridgeTool) Description() string { + if t.descriptionSuffix == "" { + return t.description + } + return t.description + t.descriptionSuffix +} func (t *BridgeTool) Parameters() map[string]any { return t.inputSchema } +// WithHints attaches admin-authored description hints to this tool. Hints are +// appended to Description() so the LLM sees server-specific quirks (e.g. "no +// trailing semicolons in code args") without modifying the upstream MCP server. +// Empty global and toolHint render no suffix. Returns t for chaining. +// +// Wire hints from MCPServerData.Settings via ParseToolHints: +// +// hints := ParseToolHints(srv.Settings) +// bt := NewBridgeTool(...).WithHints(hints.Global, hints.HintFor(mcpTool.Name)) +func (t *BridgeTool) WithHints(global, toolHint string) *BridgeTool { + g := strings.TrimSpace(global) + h := strings.TrimSpace(toolHint) + if g == "" && h == "" { + t.descriptionSuffix = "" + return t + } + var parts []string + if g != "" { + parts = append(parts, "[Server hint] "+g) + } + if h != "" { + parts = append(parts, "[Tool hint] "+h) + } + t.descriptionSuffix = "\n\n" + strings.Join(parts, "\n\n") + return t +} + // ServerName returns the name of the MCP server this tool belongs to. func (t *BridgeTool) ServerName() string { return t.serverName } diff --git a/internal/mcp/bridge_tool_test.go b/internal/mcp/bridge_tool_test.go index 7846fd8088..787862dce0 100644 --- a/internal/mcp/bridge_tool_test.go +++ b/internal/mcp/bridge_tool_test.go @@ -132,6 +132,57 @@ func TestBridgeToolNaming(t *testing.T) { } } +func TestBridgeToolWithHints(t *testing.T) { + mcpTool := mcpgo.Tool{ + Name: "search", + Description: "Run a search", + InputSchema: mcpgo.ToolInputSchema{Type: "object"}, + } + + // No hints → original description unchanged + bt := NewBridgeTool("srv", mcpTool, nil, "", 30, nil, uuid.Nil, nil) + if bt.Description() != "Run a search" { + t.Errorf("expected unchanged description, got %q", bt.Description()) + } + + // Global hint only + bt2 := NewBridgeTool("srv", mcpTool, nil, "", 30, nil, uuid.Nil, nil). + WithHints("No trailing semicolons.", "") + got := bt2.Description() + if got != "Run a search\n\n[Server hint] No trailing semicolons." { + t.Errorf("global-only mismatch:\n%q", got) + } + + // Per-tool hint only + bt3 := NewBridgeTool("srv", mcpTool, nil, "", 30, nil, uuid.Nil, nil). + WithHints("", "Use arrow func.") + if bt3.Description() != "Run a search\n\n[Tool hint] Use arrow func." { + t.Errorf("tool-only mismatch: %q", bt3.Description()) + } + + // Both hints — order: global then tool + bt4 := NewBridgeTool("srv", mcpTool, nil, "", 30, nil, uuid.Nil, nil). + WithHints("G.", "T.") + if bt4.Description() != "Run a search\n\n[Server hint] G.\n\n[Tool hint] T." { + t.Errorf("combined mismatch: %q", bt4.Description()) + } + + // Whitespace-only hints → treated as empty (no suffix) + bt5 := NewBridgeTool("srv", mcpTool, nil, "", 30, nil, uuid.Nil, nil). + WithHints(" \n ", "\t") + if bt5.Description() != "Run a search" { + t.Errorf("whitespace-only hints should render no suffix, got %q", bt5.Description()) + } + + // WithHints can be chained and reset by re-calling + bt6 := NewBridgeTool("srv", mcpTool, nil, "", 30, nil, uuid.Nil, nil). + WithHints("first", "hint") + bt6.WithHints("", "") + if bt6.Description() != "Run a search" { + t.Errorf("calling WithHints with empty should clear suffix, got %q", bt6.Description()) + } +} + func TestIsPlaceholderValue(t *testing.T) { // Should be detected as placeholder. placeholders := []string{ diff --git a/internal/mcp/manager.go b/internal/mcp/manager.go index a71f4296d8..cdfe3797d8 100644 --- a/internal/mcp/manager.go +++ b/internal/mcp/manager.go @@ -183,7 +183,8 @@ func (m *Manager) Start(ctx context.Context) error { errs = append(errs, fmt.Sprintf("%s: %v", name, err)) continue } - if err := m.connectServer(ctx, name, cfg.Transport, cfg.Command, cfg.Args, cfg.Env, cfg.URL, headers, cfg.ToolPrefix, cfg.TimeoutSec, uuid.Nil); err != nil { + // Config-path servers have no DB-backed Settings, so no tool hints. + if err := m.connectServer(ctx, name, cfg.Transport, cfg.Command, cfg.Args, cfg.Env, cfg.URL, headers, cfg.ToolPrefix, cfg.TimeoutSec, uuid.Nil, ToolHints{}); err != nil { slog.Warn("mcp.server.connect_failed", "server", name, "error", err) errs = append(errs, fmt.Sprintf("%s: %v", name, err)) } @@ -288,19 +289,20 @@ func (m *Manager) resolveServerCredentials(ctx context.Context, info store.MCPAc // and applies tool allow/deny filtering from server grants. func (m *Manager) connectAndFilter(ctx context.Context, rs *resolvedServer) error { srv := rs.info.Server + hints := ParseToolHints(srv.Settings) if m.pool != nil && !rs.hasUserCreds { // Pool mode: acquire shared connection, create per-agent BridgeTools tid := store.TenantIDFromContext(ctx) if err := m.connectViaPool(ctx, tid, srv.Name, srv.Transport, srv.Command, - rs.args, rs.env, srv.URL, rs.headers, srv.ToolPrefix, srv.TimeoutSec, srv.ID); err != nil { + rs.args, rs.env, srv.URL, rs.headers, srv.ToolPrefix, srv.TimeoutSec, srv.ID, hints); err != nil { return err } } else { // Per-agent mode: create per-agent connection if err := m.connectServer(ctx, srv.Name, srv.Transport, srv.Command, rs.args, rs.env, srv.URL, rs.headers, - srv.ToolPrefix, srv.TimeoutSec, srv.ID); err != nil { + srv.ToolPrefix, srv.TimeoutSec, srv.ID, hints); err != nil { return err } } @@ -595,3 +597,43 @@ func requireUserCreds(settings json.RawMessage) bool { _ = json.Unmarshal(settings, &s) return s.RequireUserCredentials } + +// ToolHints carries admin-authored description hints for MCP tools. +// Stored under MCPServerData.Settings.tool_hints as JSONB: +// +// { +// "tool_hints": { +// "global": "...", +// "tools": { "": "..." } +// } +// } +// +// The hints are appended to a tool's description so the LLM sees server-specific +// quirks (e.g. "no trailing semicolons in code args") without modifying the MCP +// server itself. Empty Global/Tools render no suffix. +type ToolHints struct { + Global string `json:"global,omitempty"` + Tools map[string]string `json:"tools,omitempty"` +} + +// ParseToolHints extracts tool description hints from an MCP server's Settings JSONB. +// Returns a zero-value ToolHints (no hints) if settings are empty or malformed. +// Safe to call with nil — never panics. +func ParseToolHints(settings json.RawMessage) ToolHints { + if len(settings) == 0 { + return ToolHints{} + } + var s struct { + ToolHints ToolHints `json:"tool_hints"` + } + _ = json.Unmarshal(settings, &s) + return s.ToolHints +} + +// HintFor returns the per-tool hint for toolName, or empty string if none. +func (h ToolHints) HintFor(toolName string) string { + if h.Tools == nil { + return "" + } + return h.Tools[toolName] +} diff --git a/internal/mcp/manager_connect.go b/internal/mcp/manager_connect.go index 4613a0b0a2..08cb0e27b6 100644 --- a/internal/mcp/manager_connect.go +++ b/internal/mcp/manager_connect.go @@ -101,14 +101,16 @@ func connectAndDiscover(ctx context.Context, name, transportType, command string // connectServer creates a client, initializes the connection, discovers tools, and registers them. // serverID is the MCP server UUID from DB (uuid.Nil for config-path servers). -func (m *Manager) connectServer(ctx context.Context, name, transportType, command string, args []string, env map[string]string, url string, headers map[string]string, toolPrefix string, timeoutSec int, serverID uuid.UUID) error { +// hints carries admin-authored description hints from MCPServerData.Settings.tool_hints; +// pass a zero-value ToolHints{} for config-path servers or when no hints are configured. +func (m *Manager) connectServer(ctx context.Context, name, transportType, command string, args []string, env map[string]string, url string, headers map[string]string, toolPrefix string, timeoutSec int, serverID uuid.UUID, hints ToolHints) error { ss, mcpTools, err := connectAndDiscover(ctx, name, transportType, command, args, env, url, headers, timeoutSec) if err != nil { return err } // Register tools - registeredNames := m.registerBridgeTools(ss, mcpTools, name, toolPrefix, timeoutSec, serverID) + registeredNames := m.registerBridgeTools(ss, mcpTools, name, toolPrefix, timeoutSec, serverID, hints) ss.toolNames = registeredNames // Create health monitoring context @@ -139,10 +141,12 @@ func (m *Manager) connectServer(ctx context.Context, name, transportType, comman // registerBridgeTools creates BridgeTools from MCP tool definitions and // registers them in the Manager's registry. Returns registered tool names. // serverID is the MCP server UUID (uuid.Nil for config-path servers). -func (m *Manager) registerBridgeTools(ss *serverState, mcpTools []mcpgo.Tool, serverName, toolPrefix string, timeoutSec int, serverID uuid.UUID) []string { +// hints.Global applies to all tools; hints.Tools[name] adds a per-tool hint. +func (m *Manager) registerBridgeTools(ss *serverState, mcpTools []mcpgo.Tool, serverName, toolPrefix string, timeoutSec int, serverID uuid.UUID, hints ToolHints) []string { var registeredNames []string for _, mcpTool := range mcpTools { - bt := NewBridgeTool(serverName, mcpTool, &ss.clientPtr, toolPrefix, timeoutSec, &ss.connected, serverID, m.grantChecker) + bt := NewBridgeTool(serverName, mcpTool, &ss.clientPtr, toolPrefix, timeoutSec, &ss.connected, serverID, m.grantChecker). + WithHints(hints.Global, hints.HintFor(mcpTool.Name)) if _, exists := m.registry.Get(bt.Name()); exists { slog.Warn("mcp.tool.name_collision", @@ -161,15 +165,16 @@ func (m *Manager) registerBridgeTools(ss *serverState, mcpTools []mcpgo.Tool, se // connectViaPool acquires a shared connection from the pool and creates // per-agent BridgeTools pointing to the shared client/connected pointers. -// serverID is the MCP server UUID from DB. -func (m *Manager) connectViaPool(ctx context.Context, tenantID uuid.UUID, name, transportType, command string, args []string, env map[string]string, url string, headers map[string]string, toolPrefix string, timeoutSec int, serverID uuid.UUID) error { +// serverID is the MCP server UUID from DB. hints carries admin-authored +// description hints from MCPServerData.Settings.tool_hints. +func (m *Manager) connectViaPool(ctx context.Context, tenantID uuid.UUID, name, transportType, command string, args []string, env map[string]string, url string, headers map[string]string, toolPrefix string, timeoutSec int, serverID uuid.UUID, hints ToolHints) error { entry, err := m.pool.Acquire(ctx, tenantID, name, transportType, command, args, env, url, headers, timeoutSec) if err != nil { return err } // Create per-agent BridgeTools from the pool's shared connection - registeredNames := m.registerPoolBridgeTools(entry, name, toolPrefix, timeoutSec, serverID) + registeredNames := m.registerPoolBridgeTools(entry, name, toolPrefix, timeoutSec, serverID, hints) // Track server state and per-agent tool names. // poolServers/poolToolNames keyed by plain name for Close() iteration. @@ -207,10 +212,12 @@ func (m *Manager) connectViaPool(ctx context.Context, tenantID uuid.UUID, name, // registerPoolBridgeTools creates BridgeTools from pool entry's discovered tools, // pointing to the shared client/connected pointers. Returns registered tool names. // serverID is the MCP server UUID from DB. -func (m *Manager) registerPoolBridgeTools(entry *poolEntry, serverName, toolPrefix string, timeoutSec int, serverID uuid.UUID) []string { +// hints.Global applies to all tools; hints.Tools[name] adds a per-tool hint. +func (m *Manager) registerPoolBridgeTools(entry *poolEntry, serverName, toolPrefix string, timeoutSec int, serverID uuid.UUID, hints ToolHints) []string { var registeredNames []string for _, mcpTool := range entry.tools { - bt := NewBridgeTool(serverName, mcpTool, &entry.state.clientPtr, toolPrefix, timeoutSec, &entry.state.connected, serverID, m.grantChecker) + bt := NewBridgeTool(serverName, mcpTool, &entry.state.clientPtr, toolPrefix, timeoutSec, &entry.state.connected, serverID, m.grantChecker). + WithHints(hints.Global, hints.HintFor(mcpTool.Name)) if _, exists := m.registry.Get(bt.Name()); exists { slog.Warn("mcp.tool.name_collision", diff --git a/internal/mcp/util_bm25_test.go b/internal/mcp/util_bm25_test.go index 25c3c62a77..62020c54a3 100644 --- a/internal/mcp/util_bm25_test.go +++ b/internal/mcp/util_bm25_test.go @@ -202,6 +202,64 @@ func TestRequireUserCreds_InvalidJSON(t *testing.T) { } } +// --- ParseToolHints --- + +func TestParseToolHints_Nil(t *testing.T) { + h := ParseToolHints(nil) + if h.Global != "" || len(h.Tools) != 0 { + t.Errorf("nil settings should yield empty hints, got %+v", h) + } +} + +func TestParseToolHints_Empty(t *testing.T) { + h := ParseToolHints(json.RawMessage(`{}`)) + if h.Global != "" || len(h.Tools) != 0 { + t.Errorf("empty settings should yield empty hints, got %+v", h) + } +} + +func TestParseToolHints_Full(t *testing.T) { + settings := json.RawMessage(`{ + "require_user_credentials": true, + "tool_hints": { + "global": "No trailing semicolons.", + "tools": { + "search": "Use arrow func.", + "update": "entityId must be int." + } + } + }`) + h := ParseToolHints(settings) + if h.Global != "No trailing semicolons." { + t.Errorf("global mismatch: %q", h.Global) + } + if h.HintFor("search") != "Use arrow func." { + t.Errorf("search hint mismatch: %q", h.HintFor("search")) + } + if h.HintFor("update") != "entityId must be int." { + t.Errorf("update hint mismatch: %q", h.HintFor("update")) + } + if h.HintFor("nonexistent") != "" { + t.Errorf("unknown tool should return empty string") + } +} + +func TestParseToolHints_InvalidJSON(t *testing.T) { + // Invalid JSON → zero-value hints (safe default) + h := ParseToolHints(json.RawMessage(`{invalid`)) + if h.Global != "" || len(h.Tools) != 0 { + t.Errorf("invalid JSON should yield empty hints, got %+v", h) + } +} + +func TestParseToolHints_NilHintsMap(t *testing.T) { + // HintFor must not panic when Tools map is nil + h := ToolHints{Global: "global only"} + if h.HintFor("anything") != "" { + t.Error("nil Tools map should return empty string, not panic") + } +} + // --- mcpBM25Index --- func TestMCPBM25Index_EmptyIndex(t *testing.T) { diff --git a/internal/providers/schema_normalize_test.go b/internal/providers/schema_normalize_test.go index 444724e916..59c6a588b2 100644 --- a/internal/providers/schema_normalize_test.go +++ b/internal/providers/schema_normalize_test.go @@ -508,6 +508,53 @@ func TestApplyStrictMode_NestedObject(t *testing.T) { } } +// TestApplyStrictMode_BareObjectProperty reproduces the use_skill tool +// failure: an optional property declared as `{"type":"object","description":...}` +// with NO nested `properties`. Pre-fix, applyStrictMode early-returned on +// this node (no "properties") so additionalProperties was never set, then +// makeNullable turned the type into ["object","null"], and OpenAI rejected +// with "invalid_function_parameters: 'additionalProperties' is required to +// be supplied and to be false" at path ('properties', 'params', 'type', '0'). +func TestApplyStrictMode_BareObjectProperty(t *testing.T) { + schema := map[string]any{ + "type": "object", + "properties": map[string]any{ + "name": map[string]any{"type": "string"}, + "params": map[string]any{ + "type": "object", + "description": "Optional skill-specific parameters", + }, + }, + "required": []any{"name"}, + } + result := NormalizeSchema("openai", schema) + + params := prop(result, "params") + if params == nil { + t.Fatal("expected params property to survive normalization") + } + if params["additionalProperties"] != false { + t.Errorf("bare object property must get additionalProperties:false; got %v", params["additionalProperties"]) + } + // And makeNullable should have turned type into ["object","null"]. + typ, ok := params["type"].([]any) + if !ok { + t.Fatalf("expected params.type to be a []any union, got %T: %v", params["type"], params["type"]) + } + hasObject, hasNull := false, false + for _, v := range typ { + switch v { + case "object": + hasObject = true + case "null": + hasNull = true + } + } + if !hasObject || !hasNull { + t.Errorf("expected params.type to contain both 'object' and 'null'; got %v", typ) + } +} + func TestApplyStrictMode_SkipsAnthropic(t *testing.T) { schema := map[string]any{ "type": "object", diff --git a/internal/providers/schema_strict.go b/internal/providers/schema_strict.go index 9525c8d075..03137c19b1 100644 --- a/internal/providers/schema_strict.go +++ b/internal/providers/schema_strict.go @@ -17,7 +17,20 @@ func applyStrictMode(schema map[string]any, depth int) map[string]any { typ, _ := schema["type"].(string) props, hasProps := schema["properties"].(map[string]any) - if typ != "object" || !hasProps { + if typ != "object" { + return schema + } + // Bare object schema (type:"object" with no inner "properties"). OpenAI + // strict mode still requires additionalProperties:false on such nodes — + // otherwise the later makeNullable transform turns this into + // type:["object","null"] and the strict validator rejects the null-guarded + // "object" variant for lacking additionalProperties. Set it here so tool + // authors who write `{"type":"object","description":"..."}` for a bag of + // free-form params don't produce invalid_function_parameters errors. + if !hasProps { + if _, already := schema["additionalProperties"]; !already { + schema["additionalProperties"] = false + } return schema } diff --git a/ui/web/src/components/shared/key-value-editor.tsx b/ui/web/src/components/shared/key-value-editor.tsx index 9f1a857743..486659a424 100644 --- a/ui/web/src/components/shared/key-value-editor.tsx +++ b/ui/web/src/components/shared/key-value-editor.tsx @@ -1,6 +1,7 @@ import { useState, useEffect, useRef } from "react"; import { Plus, Trash2 } from "lucide-react"; import { Input } from "@/components/ui/input"; +import { Textarea } from "@/components/ui/textarea"; import { Button } from "@/components/ui/button"; interface KeyValuePair { @@ -16,6 +17,8 @@ interface KeyValueEditorProps { addLabel?: string; /** Return true for keys whose values should be masked (type="password"). */ maskValue?: (key: string) => boolean; + /** Render value field as a single-line input (default) or multi-line textarea. */ + valueAs?: "input" | "textarea"; } function toEntries(obj: Record): KeyValuePair[] { @@ -40,6 +43,7 @@ export function KeyValueEditor({ valuePlaceholder = "Value", addLabel = "Add", maskValue, + valueAs = "input", }: KeyValueEditorProps) { const [entries, setEntries] = useState(() => toEntries(value)); const internalChange = useRef(false); @@ -78,20 +82,30 @@ export function KeyValueEditor({ return (
{entries.map((entry, idx) => ( -
+
updateEntry(idx, { key: e.target.value })} placeholder={keyPlaceholder} className="flex-1 font-mono text-sm" /> - updateEntry(idx, { value: e.target.value })} - placeholder={valuePlaceholder} - className="flex-1 font-mono text-sm" - /> + {valueAs === "textarea" ? ( +