refactor(mcp): extract registerTypedAppTool + typedAppSpec helper (KLA-419)

internal/mcp/apps.go

@@ -215,18 +215,29 @@ func (s *Server) registerAppTool(spec appSpec) {
 // common.js injected) at the ui:// URI with the MCP App MIME type. The final
 // HTML is precomputed once at registration.
 func (s *Server) registerAppResource(spec appSpec) {
-	html := renderAppHTML(spec.HTML)
+	registerUIResource(s, spec.ResourceURI, spec.ResourceName, spec.ResourceDescription, spec.HTML)
+}
+
+// registerUIResource serves an HTML body at a ui:// URI with the MCP App
+// MIME type. The HTML's appCommonMarker (if present) is replaced with
+// common.js once at registration so the per-request handler stays cheap.
+//
+// Factored out of registerAppResource so the typed-app path (see
+// registerTypedAppTool) can reuse it without recreating the appSpec
+// envelope. Pre-KLA-419 each typed app inlined the same 18 lines.
+func registerUIResource(s *Server, uri, name, description, htmlBody string) {
+	html := renderAppHTML(htmlBody)
 	s.mcpServer.AddResource(
 		&mcp.Resource{
-			URI:         spec.ResourceURI,
-			Name:        spec.ResourceName,
-			Description: spec.ResourceDescription,
+			URI:         uri,
+			Name:        name,
+			Description: description,
 			MIMEType:    mcpAppMIMEType,
 		},
 		func(ctx context.Context, req *mcp.ReadResourceRequest) (*mcp.ReadResourceResult, error) {
 			return &mcp.ReadResourceResult{
 				Contents: []*mcp.ResourceContents{{
-					URI:      spec.ResourceURI,
+					URI:      uri,
 					MIMEType: mcpAppMIMEType,
 					Text:     html,
 				}},
@@ -235,6 +246,54 @@ func (s *Server) registerAppResource(spec appSpec) {
 	)
 }
 
+// typedAppSpec mirrors appSpec for MCP Apps whose tool accepts typed
+// input parameters (e.g. user_view, device_view, insights_view). The
+// generic In type parameter is what the SDK uses to auto-derive the
+// tool's JSON input schema.
+//
+// Cannot live in a []typedAppSpec slice the way appSpecs does, because
+// each entry's In differs; instead, callers construct one literal per
+// register{Foo}View() entry point and pass it to registerTypedAppTool.
+type typedAppSpec[In any] struct {
+	Name                string
+	Description         string
+	ResourceURI         string
+	ResourceName        string
+	ResourceDescription string
+	HTML                string
+	// Handler returns the JSON-serializable payload pushed to the app as
+	// the initial tool result. The wrapper marshals it via jsonResult
+	// and surfaces errors via errorResult(spec.Name + ": <err>") so
+	// every typed app reports its tool name on failure.
+	Handler func(ctx context.Context, args In) (any, error)
+}
+
+// registerTypedAppTool is the typed-input counterpart to registerAppTool.
+// Each typed app shrinks from ~40 lines of inline registration to a
+// single typedAppSpec literal + this call. Future refactors of the
+// wrap-data-fetch / errorResult / jsonResult shape (e.g. swapping in a
+// wrapped-error pattern) become one site instead of one per app.
+func registerTypedAppTool[In any](s *Server, spec typedAppSpec[In]) {
+	meta := mcp.Meta{
+		"ui":             map[string]any{"resourceUri": spec.ResourceURI},
+		"ui/resourceUri": spec.ResourceURI, // legacy key for older hosts
+	}
+	addToolWithMetaTyped(s, spec.Name, spec.Description, meta,
+		func(ctx context.Context, req *mcp.CallToolRequest, args In) (*mcp.CallToolResult, any, error) {
+			data, err := spec.Handler(ctx, args)
+			if err != nil {
+				return errorResult(fmt.Sprintf("%s: %v", spec.Name, err)), nil, nil
+			}
+			res, err := jsonResult(data)
+			if err != nil {
+				return errorResult(err.Error()), nil, nil
+			}
+			return res, nil, nil
+		},
+	)
+	registerUIResource(s, spec.ResourceURI, spec.ResourceName, spec.ResourceDescription, spec.HTML)
+}
+
 // --- Dashboard data fetching and aggregation ---
 
 type dashboardData struct {

internal/mcp/apps_device.go

@@ -12,7 +12,6 @@ import (
 
 	"github.com/klaassen-consulting/jc/internal/api"
 	"github.com/klaassen-consulting/jc/internal/resolve"
-	"github.com/modelcontextprotocol/go-sdk/mcp"
 )
 
 //go:embed apps_html/device.html
@@ -488,49 +487,22 @@ func connectivityBucket(lastContact string, now time.Time) string {
 }
 
 // registerDeviceView wires the device_view MCP App: typed tool + ui://
-// resource. Mirrors registerUserView; lives outside appSpecs because
-// the tool takes typed input.
+// resource. Mirrors registerUserView; lives outside appSpecs because the
+// tool takes typed input.
 func (s *Server) registerDeviceView() {
-	meta := mcp.Meta{
-		"ui":             map[string]any{"resourceUri": deviceViewResourceURI},
-		"ui/resourceUri": deviceViewResourceURI,
-	}
-	addToolWithMetaTyped(s, "device_view",
-		"Show an interactive JumpCloud device inventory view: header (hostname, OS+version, serial, last contact, agent version), "+
-			"status badges (online/stale/offline, FDE, MDM), group memberships, applied policies, a system-insights snapshot "+
-			"(uptime, logged-in users, disks), and recent Directory Insights events for the device. "+
-			"Required input: device (hostname, displayName, or 24-char hex ID). "+
+	registerTypedAppTool(s, typedAppSpec[deviceViewArgs]{
+		Name: "device_view",
+		Description: "Show an interactive JumpCloud device inventory view: header (hostname, OS+version, serial, last contact, agent version), " +
+			"status badges (online/stale/offline, FDE, MDM), group memberships, applied policies, a system-insights snapshot " +
+			"(uptime, logged-in users, disks), and recent Directory Insights events for the device. " +
+			"Required input: device (hostname, displayName, or 24-char hex ID). " +
 			"Renders as a rich inventory panel in MCP App-capable hosts; returns the same data as JSON when rendering isn't supported.",
-		meta,
-		func(ctx context.Context, req *mcp.CallToolRequest, args deviceViewArgs) (*mcp.CallToolResult, any, error) {
-			data, err := fetchDeviceViewData(ctx, args)
-			if err != nil {
-				return errorResult(fmt.Sprintf("device_view: %v", err)), nil, nil
-			}
-			res, err := jsonResult(data)
-			if err != nil {
-				return errorResult(err.Error()), nil, nil
-			}
-			return res, nil, nil
-		},
-	)
-
-	rendered := renderAppHTML(deviceHTML)
-	s.mcpServer.AddResource(
-		&mcp.Resource{
-			URI:         deviceViewResourceURI,
-			Name:        "Device Inventory App",
-			Description: "Interactive JumpCloud device inventory (status, groups, policies, system insights, recent events)",
-			MIMEType:    mcpAppMIMEType,
+		ResourceURI:         deviceViewResourceURI,
+		ResourceName:        "Device Inventory App",
+		ResourceDescription: "Interactive JumpCloud device inventory (status, groups, policies, system insights, recent events)",
+		HTML:                deviceHTML,
+		Handler: func(ctx context.Context, args deviceViewArgs) (any, error) {
+			return fetchDeviceViewData(ctx, args)
 		},
-		func(ctx context.Context, req *mcp.ReadResourceRequest) (*mcp.ReadResourceResult, error) {
-			return &mcp.ReadResourceResult{
-				Contents: []*mcp.ResourceContents{{
-					URI:      deviceViewResourceURI,
-					MIMEType: mcpAppMIMEType,
-					Text:     rendered,
-				}},
-			}, nil
-		},
-	)
+	})
 }

internal/mcp/apps_insights.go

@@ -10,7 +10,6 @@ import (
 	"time"
 
 	"github.com/klaassen-consulting/jc/internal/api"
-	"github.com/modelcontextprotocol/go-sdk/mcp"
 )
 
 //go:embed apps_html/insights.html
@@ -83,48 +82,27 @@ type insightsUserCnt struct {
 }
 
 // registerInsightsView wires the insights_view MCP App: typed tool + ui://
-// resource. Lives outside appSpecs because it takes input args.
+// resource. Lives outside appSpecs because the tool takes typed input.
+//
+// Note: pre-KLA-419 the error path here was "fetching insights: <err>"
+// rather than "insights_view: <err>". The shared registerTypedAppTool
+// standardizes on the tool name for all typed apps so AI clients
+// receiving an error can correlate to the calling tool. Minor wording
+// change; no programmatic consumer should be matching on the prefix.
 func (s *Server) registerInsightsView() {
-	meta := mcp.Meta{
-		"ui":             map[string]any{"resourceUri": insightsResourceURI},
-		"ui/resourceUri": insightsResourceURI,
-	}
-	addToolWithMetaTyped(s, "insights_view",
-		"Directory Insights event explorer: stacked time-series chart by event type with top-users ranking and event preview. "+
-			"Parameters mirror `jc insights query` (service, event_type, last, start, end, user). "+
+	registerTypedAppTool(s, typedAppSpec[insightsViewArgs]{
+		Name: "insights_view",
+		Description: "Directory Insights event explorer: stacked time-series chart by event type with top-users ranking and event preview. " +
+			"Parameters mirror `jc insights query` (service, event_type, last, start, end, user). " +
 			"Renders as an interactive dashboard in MCP App-capable hosts.",
-		meta,
-		func(ctx context.Context, req *mcp.CallToolRequest, args insightsViewArgs) (*mcp.CallToolResult, any, error) {
-			data, err := fetchInsightsViewData(ctx, args)
-			if err != nil {
-				return errorResult(fmt.Sprintf("fetching insights: %v", err)), nil, nil
-			}
-			res, err := jsonResult(data)
-			if err != nil {
-				return errorResult(err.Error()), nil, nil
-			}
-			return res, nil, nil
-		},
-	)
-
-	rendered := renderAppHTML(insightsHTML)
-	s.mcpServer.AddResource(
-		&mcp.Resource{
-			URI:         insightsResourceURI,
-			Name:        "Insights Explorer",
-			Description: "Interactive Directory Insights time-series and top-users view",
-			MIMEType:    mcpAppMIMEType,
+		ResourceURI:         insightsResourceURI,
+		ResourceName:        "Insights Explorer",
+		ResourceDescription: "Interactive Directory Insights time-series and top-users view",
+		HTML:                insightsHTML,
+		Handler: func(ctx context.Context, args insightsViewArgs) (any, error) {
+			return fetchInsightsViewData(ctx, args)
 		},
-		func(ctx context.Context, req *mcp.ReadResourceRequest) (*mcp.ReadResourceResult, error) {
-			return &mcp.ReadResourceResult{
-				Contents: []*mcp.ResourceContents{{
-					URI:      insightsResourceURI,
-					MIMEType: mcpAppMIMEType,
-					Text:     rendered,
-				}},
-			}, nil
-		},
-	)
+	})
 }
 
 // resolveInsightsWindow turns args into a concrete [start, end] pair in UTC,

internal/mcp/apps_test.go

@@ -509,3 +509,33 @@ func TestAppSpecs_UniqueNamesAndURIs(t *testing.T) {
 		uris[spec.ResourceURI] = true
 	}
 }
+
+// KLA-419 introduced registerTypedAppTool to deduplicate the user_view /
+// device_view / insights_view registration boilerplate. The other
+// existing tests (TestUserView_HasUIMetadata,
+// TestUserViewResource_ServesHTMLWithInjection, and their device + insights
+// counterparts) cover the happy path through the helper end-to-end.
+//
+// This test pins the error-path contract: a typed app's errorResult must
+// prefix the message with the tool name. Pre-KLA-419 the insights_view
+// path used "fetching insights:" — drift-prone and inconsistent with
+// user_view / device_view. The helper now standardizes on
+// spec.Name + ": <err>" so AI clients can correlate the error back to
+// the calling tool.
+func TestRegisterTypedAppTool_ErrorMessagePrefixedWithToolName(t *testing.T) {
+	setupToolTest(t)
+	cs := connectToolTestServer(t, Options{})
+
+	// insights_view with an unparseable "last" shorthand reaches the
+	// handler (passes SDK input validation), then parseInsightsTime
+	// returns a "parsing last: ..." error. The wrapper must prefix it
+	// with "insights_view: ".
+	result := callTool(t, cs, "insights_view", map[string]any{"last": "garbage-duration"})
+	if !result.IsError {
+		t.Fatal("expected error for unparseable insights_view last; got success")
+	}
+	text := getResultText(t, result)
+	if !strings.HasPrefix(text, "insights_view:") {
+		t.Errorf("insights_view error should be prefixed with %q, got: %q", "insights_view:", text)
+	}
+}

internal/mcp/apps_user.go

@@ -12,7 +12,6 @@ import (
 
 	"github.com/klaassen-consulting/jc/internal/api"
 	"github.com/klaassen-consulting/jc/internal/resolve"
-	"github.com/modelcontextprotocol/go-sdk/mcp"
 )
 
 //go:embed apps_html/user.html
@@ -317,45 +316,20 @@ func previewSSHKey(pub string) string {
 }
 
 // registerUserView wires the user_view MCP App: typed tool + ui:// resource.
-// Lives outside appSpecs because it takes input args.
+// Lives outside appSpecs because the tool takes typed input; uses
+// registerTypedAppTool to share the wrap-fetch / errorResult / jsonResult
+// + resource-handler boilerplate with the other typed apps.
 func (s *Server) registerUserView() {
-	meta := mcp.Meta{
-		"ui":             map[string]any{"resourceUri": userViewResourceURI},
-		"ui/resourceUri": userViewResourceURI,
-	}
-	addToolWithMetaTyped(s, "user_view",
-		"Show an interactive JumpCloud user profile: header (username, email, status badges), MFA enrollment, group memberships, SSH keys, and recent auth events. "+
+	registerTypedAppTool(s, typedAppSpec[userViewArgs]{
+		Name: "user_view",
+		Description: "Show an interactive JumpCloud user profile: header (username, email, status badges), MFA enrollment, group memberships, SSH keys, and recent auth events. " +
 			"Required input: user (username, email, or ID). Renders as a rich profile in MCP App-capable hosts; returns the same data as JSON when rendering isn't supported.",
-		meta,
-		func(ctx context.Context, req *mcp.CallToolRequest, args userViewArgs) (*mcp.CallToolResult, any, error) {
-			data, err := fetchUserViewData(ctx, args)
-			if err != nil {
-				return errorResult(fmt.Sprintf("user_view: %v", err)), nil, nil
-			}
-			res, err := jsonResult(data)
-			if err != nil {
-				return errorResult(err.Error()), nil, nil
-			}
-			return res, nil, nil
-		},
-	)
-
-	rendered := renderAppHTML(userHTML)
-	s.mcpServer.AddResource(
-		&mcp.Resource{
-			URI:         userViewResourceURI,
-			Name:        "User Profile App",
-			Description: "Interactive JumpCloud user profile (groups, SSH keys, MFA, recent events)",
-			MIMEType:    mcpAppMIMEType,
+		ResourceURI:         userViewResourceURI,
+		ResourceName:        "User Profile App",
+		ResourceDescription: "Interactive JumpCloud user profile (groups, SSH keys, MFA, recent events)",
+		HTML:                userHTML,
+		Handler: func(ctx context.Context, args userViewArgs) (any, error) {
+			return fetchUserViewData(ctx, args)
 		},
-		func(ctx context.Context, req *mcp.ReadResourceRequest) (*mcp.ReadResourceResult, error) {
-			return &mcp.ReadResourceResult{
-				Contents: []*mcp.ResourceContents{{
-					URI:      userViewResourceURI,
-					MIMEType: mcpAppMIMEType,
-					Text:     rendered,
-				}},
-			}, nil
-		},
-	)
+	})
 }