Promote deferred MCP tools on a registry-miss instead of 404ing

cmd/gortex/daemon.go

@@ -409,6 +409,7 @@ func runDaemonStart(cmd *cobra.Command, _ []string) error {
 		// handler needs — the MCP server, graph, config manager, overlay
 		// manager, and federation router — so this is pure composition.
 		v1 := server.NewHandler(state.mcpServer.MCPServer(), state.graph, version, logger)
+
 		if state.configManager != nil {
 			v1.SetConfigManager(state.configManager)
 		}

cmd/gortex/daemon_mcp.go

@@ -445,7 +445,18 @@ func (d *mcpDispatcher) tryProxyToolCall(ctx context.Context, sess *daemon.Sessi
 		return nil, false
 	}
 	scope, _ := peek.Params.Arguments["workspace"].(string)
-	body, err := json.Marshal(map[string]any{"arguments": peek.Params.Arguments})
+	// A no-arguments call (MCP allows omitting params.arguments) leaves
+	// peek.Params.Arguments nil, which json.Marshal renders as literal
+	// `"arguments":null` — indistinguishable, to a body-shape validator,
+	// from a malformed caller-sent null. Normalize to an empty object so
+	// the executor's "arguments must be an object when present" check
+	// (added for reviewer concern #2) never rejects a legitimate no-arg
+	// call.
+	args := peek.Params.Arguments
+	if args == nil {
+		args = map[string]any{}
+	}
+	body, err := json.Marshal(map[string]any{"arguments": args})
 	if err != nil {
 		return nil, false
 	}

cmd/gortex/server_router.go

@@ -10,6 +10,7 @@ import (
 
 	"github.com/zzet/gortex/internal/daemon"
 	gortexmcp "github.com/zzet/gortex/internal/mcp"
+	"github.com/zzet/gortex/internal/server"
 )
 
 // newLocalToolExecutor builds the daemon.LocalExecutor closure used by
@@ -32,7 +33,80 @@ func newLocalToolExecutor(srv *gortexmcp.Server, logger *zap.Logger) daemon.Loca
 		}
 	}
 	return func(ctx context.Context, toolName string, body []byte) ([]byte, int, error) {
+		// Validate the request body before any lookup, promotion, or
+		// invocation: malformed JSON must 400 without touching the
+		// registry or running a handler. A JSON-null body (top-level
+		// `null` or `{"arguments": null}`) is rejected explicitly —
+		// json.Unmarshal treats null as a silent no-op for both struct
+		// and map targets, so it would otherwise sail through as "no
+		// arguments" instead of being flagged as malformed input.
+		var args map[string]any
+		if len(body) > 0 {
+			var probe any
+			if err := json.Unmarshal(body, &probe); err != nil {
+				payload := map[string]any{
+					"error":   "invalid_json",
+					"message": fmt.Sprintf("malformed request body: %s", err.Error()),
+				}
+				out, _ := json.Marshal(payload)
+				return out, 400, nil
+			}
+			obj, ok := probe.(map[string]any)
+			if !ok {
+				payload := map[string]any{
+					"error":   "invalid_json",
+					"message": "malformed request body: expected a JSON object",
+				}
+				out, _ := json.Marshal(payload)
+				return out, 400, nil
+			}
+			if rawArgs, present := obj["arguments"]; present {
+				nested, ok := rawArgs.(map[string]any)
+				if !ok {
+					payload := map[string]any{
+						"error":   "invalid_json",
+						"message": `malformed request body: "arguments" must be a JSON object`,
+					}
+					out, _ := json.Marshal(payload)
+					return out, 400, nil
+				}
+				args = nested
+			} else {
+				args = obj
+			}
+		}
+
+		// An already-live tool (whether generally allowed or blocked by
+		// the session's active preset/facade surface) dispatches
+		// straight to its handler with NO gate here: every production
+		// registration path (addTool, addControlTool, lazy promote,
+		// facade_tools) wraps the handler with wrapToolHandlerMode,
+		// which runs checkToolGate on every call — including this one,
+		// now that ctx carries the caller's session id (see the
+		// handleToolCall / tryRouteToolCall ctx-ordering fix). That gate
+		// is what should decide a blocked-by-preset call: it returns a
+		// structured tool_blocked_by_mode error the client can act on
+		// (which preset, how to reconnect). Adding a coarser gate here
+		// too previously collapsed that structured error into a bare
+		// 404 "not found" — a lie for a tool that IS registered — so
+		// this path deliberately does not duplicate the check for an
+		// already-live tool.
+		//
+		// A NOT-yet-live (deferred) tool is different: promoting it is
+		// itself a side effect (it mutates the shared lazy registry
+		// process-wide), so that side effect must stay gated on the
+		// session's effective surface — EnsureToolPromotedForSession
+		// checks IsToolEnabledForSession before promoting, so a session
+		// whose surface hides the tool never promotes it (and gets a
+		// 404, since there's nothing live to dispatch to and nothing to
+		// promote on its behalf).
 		tool := srv.MCPServer().GetTool(toolName)
+		if tool == nil {
+			if srv.EnsureToolPromotedForSession(ctx, toolName) {
+				ctx = gortexmcp.WithAuthorizedToolCall(ctx, toolName)
+				tool = srv.MCPServer().GetTool(toolName)
+			}
+		}
 		if tool == nil {
 			payload := map[string]any{
 				"error":   "tool_not_found",
@@ -42,18 +116,6 @@ func newLocalToolExecutor(srv *gortexmcp.Server, logger *zap.Logger) daemon.Loca
 			return out, 404, nil
 		}
 
-		var args map[string]any
-		if len(body) > 0 {
-			var nested struct {
-				Arguments map[string]any `json:"arguments"`
-			}
-			if err := json.Unmarshal(body, &nested); err == nil && nested.Arguments != nil {
-				args = nested.Arguments
-			} else {
-				_ = json.Unmarshal(body, &args)
-			}
-		}
-
 		mcpReq := mcp.CallToolRequest{
 			Params: mcp.CallToolParams{
 				Name:      toolName,
@@ -72,17 +134,24 @@ func newLocalToolExecutor(srv *gortexmcp.Server, logger *zap.Logger) daemon.Loca
 			return out, 500, nil
 		}
 
-		// Mirror the same response shape the HTTP handler emits so
-		// the proxy and local paths are indistinguishable downstream.
-		resp := struct {
-			IsError bool             `json:"is_error,omitempty"`
-			Content []map[string]any `json:"content,omitempty"`
-		}{IsError: result.IsError}
+		// Reuse the SAME response type internal/server's HTTP handler
+		// and the Streamable HTTP transport's wrapToolResultAsJSONRPC
+		// both serialize/parse (server.ToolResponse: "content"/
+		// "isError") — not an independently-typed lookalike. A prior
+		// version of this struct tagged the error field "is_error"
+		// (snake_case); wrapToolResultAsJSONRPC only recognizes
+		// "isError" and silently defaults IsError to false on a
+		// mismatch, so a genuine tool error routed through the
+		// Streamable HTTP transport's local-fast path was reported to
+		// the client as a successful result. Sharing one type makes
+		// that class of drift a compile error instead of a silent
+		// wire-format bug.
+		resp := server.ToolResponse{IsError: result.IsError}
 		for _, c := range result.Content {
 			if tc, ok := c.(mcp.TextContent); ok {
-				resp.Content = append(resp.Content, map[string]any{
-					"type": "text",
-					"text": tc.Text,
+				resp.Content = append(resp.Content, server.ToolContent{
+					Type: "text",
+					Text: tc.Text,
 				})
 			}
 		}