diff --git a/internal/mcp/apps.go b/internal/mcp/apps.go index b2182b3..9f58c5c 100644 --- a/internal/mcp/apps.go +++ b/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 + ": ") 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 { diff --git a/internal/mcp/apps_device.go b/internal/mcp/apps_device.go index 2e3f0a3..fa69cdc 100644 --- a/internal/mcp/apps_device.go +++ b/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 - }, - ) + }) } diff --git a/internal/mcp/apps_insights.go b/internal/mcp/apps_insights.go index 2e664d8..e49f12b 100644 --- a/internal/mcp/apps_insights.go +++ b/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: " +// rather than "insights_view: ". 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, diff --git a/internal/mcp/apps_test.go b/internal/mcp/apps_test.go index 4e832c9..24ee0b9 100644 --- a/internal/mcp/apps_test.go +++ b/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 + ": " 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) + } +} diff --git a/internal/mcp/apps_user.go b/internal/mcp/apps_user.go index ec02647..5766e1b 100644 --- a/internal/mcp/apps_user.go +++ b/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 - }, - ) + }) }