From 95521e64c3cf6b06592f5bb461d965711181af92 Mon Sep 17 00:00:00 2001 From: tech-synity Date: Tue, 21 Apr 2026 20:57:05 +0700 Subject: [PATCH 1/9] fix(build): anchor pkg-helper gitignore rule to repo root MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The bare pattern `pkg-helper` on line 6 matched any file or directory named pkg-helper anywhere in the tree — including the tracked source directory `cmd/pkg-helper/`. Git tolerated this because already-tracked files override ignore rules, but uploaders that apply .gitignore patterns naively to the filesystem (notably `railway up`) stripped the source directory from the build context, breaking `go build ./cmd/pkg-helper` in Docker with `stat /src/cmd/pkg-helper: directory not found`. Anchor both binary-artifact patterns (`/openclaw-go`, `/pkg-helper`) to the repo root so they only match the compiled output, not the source. Co-Authored-By: Claude Opus 4.7 --- .gitignore | 9 ++++++--- 1 file changed, 6 insertions(+), 3 deletions(-) diff --git a/.gitignore b/.gitignore index 85435cf052..da42d940d9 100644 --- a/.gitignore +++ b/.gitignore @@ -1,9 +1,12 @@ # 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 # IDE .idea/ From 65afb48415a7bd323b509c518f52d62cb84e1672 Mon Sep 17 00:00:00 2001 From: tech-synity Date: Wed, 22 Apr 2026 17:55:50 +0700 Subject: [PATCH 2/9] chore: ignore local build artifacts + Railway-exclude debug probe dirs MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Working tree kept surfacing `goclaw.exe` / `goclaw-local.exe` (Windows dev builds) and ad-hoc `tmp-reset-bot/` probe scripts as untracked. A single `git add .` would commit them — both are useless outside the author's machine and the debug probes import `internal/` packages from outside `cmd/` (ugly but legal), so they should never ride into Git. Add to .gitignore (anchored like the existing `/openclaw-go` entry, so matching doesn't accidentally touch source dirs — same concern that broke Docker builds in 2de99e26): /goclaw.exe /goclaw-local.exe /tmp-reset-bot/ /tmp-probe*/ Also add .railwayignore file — new; Railway copies the working tree verbatim and already ignored tests/node_modules/etc. Add `tmp-*` glob so ad-hoc probe dirs don't bloat image layers on `railway up`. Co-Authored-By: Claude Opus 4.7 --- .gitignore | 7 +++++++ .railwayignore | 18 ++++++++++++++++++ 2 files changed, 25 insertions(+) create mode 100644 .railwayignore diff --git a/.gitignore b/.gitignore index da42d940d9..5b7250cff2 100644 --- a/.gitignore +++ b/.gitignore @@ -7,6 +7,13 @@ tests/integration/testdata/ # — 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 +._* From f776b2a2388e7dec3d710220ddaff6ea294d32a3 Mon Sep 17 00:00:00 2001 From: tech-synity Date: Wed, 22 Apr 2026 17:55:58 +0700 Subject: [PATCH 3/9] =?UTF-8?q?chore(deps):=20bump=20wailsapp/wails=20v2.1?= =?UTF-8?q?1.0=20=E2=86=92=20v2.12.0?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Routine dependency refresh. v2.12 pulls in a new indirect dep (git.sr.ht/~jackmordaunt/go-toast/v2 v2.0.3 — used by Wails' notification path) and otherwise contains only maintenance fixes from upstream. Verified: `go build -tags sqliteonly ./internal/... ./cmd/` green — desktop edition's Wails bindings still compile. No goclaw code calls the notification API directly, so the new indirect is latent unless Wails decides to pull it into the main path later. Co-Authored-By: Claude Opus 4.7 --- go.mod | 3 ++- go.sum | 10 ++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) 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= From 7ee1226f2ba66c0fc8957a6ec08bf913362933e1 Mon Sep 17 00:00:00 2001 From: tech-synity Date: Wed, 22 Apr 2026 17:56:10 +0700 Subject: [PATCH 4/9] fix(providers): set additionalProperties:false on bare object schemas in strict mode MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit OpenAI strict mode rejected `use_skill` calls with: invalid_function_parameters: 'additionalProperties' is required to be supplied and to be false at path ('properties', 'params', 'type', '0') Root cause: applyStrictMode early-returned when a node was `{"type":"object"}` with no nested `properties` key, leaving additionalProperties unset. Then makeNullable widened the type to `["object","null"]`, and the strict validator checks each branch independently — the "object" variant was missing additionalProperties so the whole schema failed validation. Fix: when typ=="object", set additionalProperties:false even if the node has no inner properties. This covers the common "bag of free-form params" shape tool authors write as: params: { type: "object", description: "..." } Trade-off worth documenting: strict mode + bare object ⇒ OpenAI will reject any call that actually populates those params. If a tool truly needs a free-form dict of arbitrary keys, strict mode is the wrong fit — either declare the known keys under `properties`, or opt that tool out of strict. Left a comment on the branch spelling this out. Regression test added: TestApplyStrictMode_BareObjectProperty asserts both `additionalProperties:false` AND the `["object","null"]` type union survive the transform. Co-Authored-By: Claude Opus 4.7 --- internal/providers/schema_normalize_test.go | 47 +++++++++++++++++++++ internal/providers/schema_strict.go | 15 ++++++- 2 files changed, 61 insertions(+), 1 deletion(-) 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 } From dd5930c669d1525e1987577b87940a5d25767d26 Mon Sep 17 00:00:00 2001 From: tech-synity Date: Tue, 21 Apr 2026 21:17:03 +0700 Subject: [PATCH 5/9] feat(gateway): serve JSON index at / when no embedded UI MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Previously, builds without the `embedui` tag left "/" unhandled, so http.ServeMux returned a bare 404 when an operator opened the deployed URL in a browser. That's needlessly opaque — it looks like the service is broken even though the gateway is healthy. Register a small JSON index handler for "/" in the no-UI branch. It returns service status + protocol version + a list of real endpoints for an exact "/" match, and a JSON 404 for any other unmatched path (http.ServeMux routes "/" as a catch-all). Embedui builds are unchanged — the webui.Handler() still owns "/" and takes precedence. Co-Authored-By: Claude Opus 4.7 --- internal/gateway/server.go | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) 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 != "" { From 43f798dbec0673d8ce128185b4e68bb7d46f970b Mon Sep 17 00:00:00 2001 From: tech-synity Date: Thu, 23 Apr 2026 23:35:16 +0700 Subject: [PATCH 6/9] feat(mcp): admin-authored tool description hints via Settings.tool_hints Lets admins attach server-specific quirks (e.g. "no trailing semicolons in code args") to MCP tool descriptions without modifying the upstream MCP server or redeploying goclaw. Hints are stored in the existing mcp_servers.settings JSONB (no migration) following the same pattern as require_user_credentials. Backend: - Add mcp.ToolHints type + ParseToolHints() helper alongside requireUserCreds - Add BridgeTool.WithHints(global, toolHint) setter that appends a suffix to Description() so the LLM sees hints as part of the tool schema - Thread hints through connectServer / connectViaPool / loop_mcp_user - 12 new unit tests for ParseToolHints + WithHints (+ edge cases) Frontend: - New "AI Agent Hints (Optional)" section in MCP form dialog: a global textarea + a key-value list for per-tool hints - Extend KeyValueEditor with valueAs="textarea" prop (reusable) - Zod schema + TS types + i18n (en/vi/zh) updated Co-Authored-By: Claude Opus 4.7 --- internal/agent/loop_mcp_user.go | 4 +- internal/mcp/bridge_tool.go | 59 +++++++++++++++---- internal/mcp/bridge_tool_test.go | 51 ++++++++++++++++ internal/mcp/manager.go | 48 ++++++++++++++- internal/mcp/manager_connect.go | 25 +++++--- internal/mcp/util_bm25_test.go | 58 ++++++++++++++++++ .../components/shared/key-value-editor.tsx | 30 +++++++--- ui/web/src/i18n/locales/en/mcp.json | 11 +++- ui/web/src/i18n/locales/vi/mcp.json | 11 +++- ui/web/src/i18n/locales/zh/mcp.json | 11 +++- ui/web/src/pages/mcp/mcp-form-dialog.tsx | 26 +++++++- ui/web/src/pages/mcp/mcp-settings-fields.tsx | 40 +++++++++++++ ui/web/src/schemas/mcp.schema.ts | 4 ++ ui/web/src/types/mcp.ts | 16 ++++- 14 files changed, 354 insertions(+), 40 deletions(-) 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/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/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" ? ( +