-
Notifications
You must be signed in to change notification settings - Fork 6
feat: improve error handling and validation in skills manifest and related files #316
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Merged
Merged
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
965fde6
feat: improve error handling and validation in skills manifest and re…
frontegg-david 27db74e
feat: add frontmatter metadata to various documentation files for imp…
frontegg-david 7e5065a
refactor: clean up code by removing unnecessary comments and improvin…
frontegg-david 2ba5d57
refactor: clean up code by removing unnecessary comments and improvin…
frontegg-david 7863e28
refactor: simplify debug provider tests and remove unnecessary proper…
frontegg-david c805ccc
refactor: simplify debug provider tests and remove unnecessary proper…
frontegg-david File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
173 changes: 173 additions & 0 deletions
173
apps/e2e/demo-e2e-resource-providers/e2e/resource-providers.e2e.spec.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,173 @@ | ||
| /** | ||
| * E2E Tests: Resource Provider Resolution & Plugin Context Extensions | ||
| * | ||
| * Verifies that: | ||
| * 1. App-level providers registered via @App({ providers: [...] }) are accessible | ||
| * from resources via this.get(Token), sharing the same GLOBAL singleton as tools. | ||
| * 2. Plugin providers exposed via contextExtensions (e.g., this.counter) work | ||
| * correctly in resource contexts, not just tool contexts. | ||
| */ | ||
| import { test, expect } from '@frontmcp/testing'; | ||
|
|
||
| /** | ||
| * Extract JSON content from a resource read result. | ||
| * Resources return { contents: [{ text: '...' }] } so we parse the text. | ||
| */ | ||
| function extractResourceJson<T>(result: unknown): T { | ||
| const raw = result as { raw?: { contents?: Array<{ text?: string }> } }; | ||
| const text = raw?.raw?.contents?.[0]?.text; | ||
| if (!text) throw new Error('No text content in resource result'); | ||
| return JSON.parse(text) as T; | ||
| } | ||
|
|
||
| /** | ||
| * Extract structured content from a tool call result. | ||
| */ | ||
| function extractToolJson<T>(result: unknown): T { | ||
| const raw = result as { raw?: { structuredContent?: T; content?: Array<{ text?: string }> } }; | ||
| if (raw?.raw?.structuredContent) return raw.raw.structuredContent; | ||
| const text = raw?.raw?.content?.[0]?.text; | ||
| if (!text) throw new Error('No content in tool result'); | ||
| return JSON.parse(text) as T; | ||
| } | ||
|
|
||
| test.describe('Resource Provider Resolution E2E', () => { | ||
| test.use({ | ||
| server: 'apps/e2e/demo-e2e-resource-providers/src/main.ts', | ||
| project: 'demo-e2e-resource-providers', | ||
| publicMode: true, | ||
| }); | ||
|
|
||
| // ─── Discovery ────────────────────────────────────────────────────────── | ||
|
|
||
| test.describe('Discovery', () => { | ||
| test('should list all tools', async ({ mcp }) => { | ||
| const tools = await mcp.tools.list(); | ||
| expect(tools).toContainTool('store_set'); | ||
| expect(tools).toContainTool('store_get'); | ||
| expect(tools).toContainTool('counter_increment'); | ||
| expect(tools).toContainTool('debug_providers'); | ||
| }); | ||
|
|
||
| test('should list all resources', async ({ mcp }) => { | ||
| const resources = await mcp.resources.list(); | ||
| expect(resources).toContainResource('store://contents'); | ||
| expect(resources).toContainResource('counter://status'); | ||
| expect(resources).toContainResource('debug://providers'); | ||
| }); | ||
| }); | ||
|
|
||
| // ─── App-level provider in resource via this.get() ───────────────────── | ||
|
|
||
| test.describe('App Provider in Resource', () => { | ||
| test('tool can resolve app provider via this.get()', async ({ mcp }) => { | ||
| const result = await mcp.tools.call('store_set', { key: 'test', value: 'hello' }); | ||
| expect(result).toBeSuccessful(); | ||
| expect(result).toHaveTextContent('storeInstanceId'); | ||
| }); | ||
|
|
||
| test('resource can resolve same app provider via this.get()', async ({ mcp }) => { | ||
| const resource = await mcp.resources.read('store://contents'); | ||
| expect(resource).toBeSuccessful(); | ||
| expect(resource).toHaveTextContent('storeInstanceId'); | ||
| }); | ||
|
|
||
| test('resource and tool share the same GLOBAL provider instance', async ({ mcp }) => { | ||
| // Store a value via tool | ||
| const setResult = await mcp.tools.call('store_set', { key: 'shared-test', value: 'from-tool' }); | ||
| expect(setResult).toBeSuccessful(); | ||
|
|
||
| // Read back via resource — should see the same data (same singleton) | ||
| const resource = await mcp.resources.read('store://contents'); | ||
| expect(resource).toBeSuccessful(); | ||
| expect(resource).toHaveTextContent('shared-test'); | ||
| expect(resource).toHaveTextContent('from-tool'); | ||
|
|
||
| // Compare storeInstanceId | ||
| const toolData = extractToolJson<{ storeInstanceId: string }>(setResult); | ||
| const resourceData = extractResourceJson<{ storeInstanceId: string }>(resource); | ||
|
|
||
| expect(toolData.storeInstanceId).toBeDefined(); | ||
| expect(resourceData.storeInstanceId).toBeDefined(); | ||
| expect(toolData.storeInstanceId).toBe(resourceData.storeInstanceId); | ||
| }); | ||
|
|
||
| test('resource sees data written by tool (shared state)', async ({ mcp }) => { | ||
| await mcp.tools.call('store_set', { key: 'cross-check', value: 'works' }); | ||
| const resource = await mcp.resources.read('store://contents'); | ||
| expect(resource).toBeSuccessful(); | ||
| expect(resource).toHaveTextContent('cross-check'); | ||
|
|
||
| const getResult = await mcp.tools.call('store_get', { key: 'cross-check' }); | ||
| expect(getResult).toBeSuccessful(); | ||
| expect(getResult).toHaveTextContent('works'); | ||
| }); | ||
| }); | ||
|
|
||
| // ─── Plugin context extension in resource ────────────────────────────── | ||
|
|
||
| test.describe('Plugin Context Extension in Resource', () => { | ||
| test('tool can access plugin context extension (this.counter)', async ({ mcp }) => { | ||
| const result = await mcp.tools.call('counter_increment', {}); | ||
| expect(result).toBeSuccessful(); | ||
| expect(result).toHaveTextContent('counterInstanceId'); | ||
| }); | ||
|
|
||
| test('resource can access plugin context extension (this.counter)', async ({ mcp }) => { | ||
| const resource = await mcp.resources.read('counter://status'); | ||
| expect(resource).toBeSuccessful(); | ||
| expect(resource).toHaveTextContent('counterInstanceId'); | ||
| }); | ||
|
|
||
| test('resource and tool share same plugin provider instance', async ({ mcp }) => { | ||
| // Increment via tool | ||
| const inc1 = await mcp.tools.call('counter_increment', {}); | ||
| expect(inc1).toBeSuccessful(); | ||
| const inc2 = await mcp.tools.call('counter_increment', {}); | ||
| expect(inc2).toBeSuccessful(); | ||
|
|
||
| // Read counter status via resource | ||
| const resource = await mcp.resources.read('counter://status'); | ||
| expect(resource).toBeSuccessful(); | ||
|
|
||
| // Counter was incremented twice, so count should be >= 2 | ||
| const resourceData = extractResourceJson<{ count: number; counterInstanceId: string }>(resource); | ||
| expect(resourceData.count).toBeGreaterThanOrEqual(2); | ||
|
|
||
| // Verify same plugin instance | ||
| const toolData = extractToolJson<{ counterInstanceId: string }>(inc1); | ||
| expect(toolData.counterInstanceId).toBe(resourceData.counterInstanceId); | ||
| }); | ||
| }); | ||
|
|
||
| // ─── Cross-component consistency ─────────────────────────────────────── | ||
|
|
||
| test.describe('Cross-Component Provider Consistency', () => { | ||
| test('multiple resource reads use same provider instance', async ({ mcp }) => { | ||
| const res1 = await mcp.resources.read('store://contents'); | ||
| const res2 = await mcp.resources.read('store://contents'); | ||
|
|
||
| expect(res1).toBeSuccessful(); | ||
| expect(res2).toBeSuccessful(); | ||
|
|
||
| const data1 = extractResourceJson<{ storeInstanceId: string }>(res1); | ||
| const data2 = extractResourceJson<{ storeInstanceId: string }>(res2); | ||
| expect(data1.storeInstanceId).toBe(data2.storeInstanceId); | ||
| }); | ||
|
|
||
| test('debug tool and resource resolve same provider instance', async ({ mcp }) => { | ||
| const toolDebug = await mcp.tools.call('debug_providers', {}); | ||
| const resourceDebug = await mcp.resources.read('debug://providers'); | ||
|
|
||
| expect(toolDebug).toBeSuccessful(); | ||
| expect(resourceDebug).toBeSuccessful(); | ||
|
|
||
| const toolData = extractToolJson<{ storeInstanceId: string }>(toolDebug); | ||
| const resourceData = extractResourceJson<{ storeInstanceId: string }>(resourceDebug); | ||
|
|
||
| expect(toolData.storeInstanceId).toBeDefined(); | ||
| expect(resourceData.storeInstanceId).toBeDefined(); | ||
| expect(toolData.storeInstanceId).toBe(resourceData.storeInstanceId); | ||
| }); | ||
| }); | ||
| }); | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,44 @@ | ||
| import type { Config } from '@jest/types'; | ||
| import { createRequire } from 'module'; | ||
|
|
||
| const require = createRequire(import.meta.url); | ||
| const e2eCoveragePreset = require('../../../jest.e2e.coverage.preset.js'); | ||
|
|
||
| const config: Config.InitialOptions = { | ||
| displayName: 'demo-e2e-resource-providers', | ||
| preset: '../../../jest.preset.js', | ||
| testEnvironment: 'node', | ||
| testMatch: ['<rootDir>/e2e/**/*.e2e.spec.ts'], | ||
| testTimeout: 60000, | ||
| maxWorkers: 1, | ||
| setupFilesAfterEnv: ['<rootDir>/../../../libs/testing/src/setup.ts'], | ||
| transformIgnorePatterns: ['node_modules/(?!(jose)/)'], | ||
| transform: { | ||
| '^.+\\.[tj]s$': [ | ||
| '@swc/jest', | ||
| { | ||
| jsc: { | ||
| parser: { | ||
| syntax: 'typescript', | ||
| decorators: true, | ||
| }, | ||
| transform: { | ||
| decoratorMetadata: true, | ||
| }, | ||
| target: 'es2022', | ||
| }, | ||
| }, | ||
| ], | ||
| }, | ||
| moduleNameMapper: { | ||
| '^@frontmcp/testing$': '<rootDir>/../../../libs/testing/src/index.ts', | ||
| '^@frontmcp/sdk$': '<rootDir>/../../../libs/sdk/src/index.ts', | ||
| '^@frontmcp/adapters$': '<rootDir>/../../../libs/adapters/src/index.ts', | ||
| '^@frontmcp/auth$': '<rootDir>/../../../libs/auth/src/index.ts', | ||
| '^@frontmcp/utils$': '<rootDir>/../../../libs/utils/src/index.ts', | ||
| }, | ||
| coverageDirectory: '../../../coverage/e2e/demo-e2e-resource-providers', | ||
| ...e2eCoveragePreset, | ||
| }; | ||
|
|
||
| export default config; |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,54 @@ | ||
| { | ||
| "name": "demo-e2e-resource-providers", | ||
| "$schema": "../../../node_modules/nx/schemas/project-schema.json", | ||
| "sourceRoot": "apps/e2e/demo-e2e-resource-providers/src", | ||
| "projectType": "application", | ||
| "tags": ["scope:demo", "type:e2e", "feature:resource-providers"], | ||
| "targets": { | ||
| "build": { | ||
| "executor": "@nx/webpack:webpack", | ||
| "outputs": ["{options.outputPath}"], | ||
| "defaultConfiguration": "development", | ||
| "options": { | ||
| "target": "node", | ||
| "compiler": "tsc", | ||
| "outputPath": "dist/apps/e2e/demo-e2e-resource-providers", | ||
| "main": "apps/e2e/demo-e2e-resource-providers/src/main.ts", | ||
| "tsConfig": "apps/e2e/demo-e2e-resource-providers/tsconfig.app.json", | ||
| "webpackConfig": "apps/e2e/demo-e2e-resource-providers/webpack.config.js", | ||
| "generatePackageJson": true | ||
| }, | ||
| "configurations": { | ||
| "development": {}, | ||
| "production": { | ||
| "optimization": true | ||
| } | ||
| } | ||
| }, | ||
| "serve": { | ||
| "executor": "nx:run-commands", | ||
| "dependsOn": ["build"], | ||
| "options": { | ||
| "command": "node dist/apps/e2e/demo-e2e-resource-providers/main.js", | ||
| "cwd": "{workspaceRoot}" | ||
| } | ||
| }, | ||
| "test": { | ||
| "executor": "@nx/jest:jest", | ||
| "outputs": ["{workspaceRoot}/coverage/apps/e2e/demo-e2e-resource-providers"], | ||
| "options": { | ||
| "jestConfig": "apps/e2e/demo-e2e-resource-providers/jest.e2e.config.ts", | ||
| "passWithNoTests": true | ||
| } | ||
| }, | ||
| "test:e2e": { | ||
| "executor": "@nx/jest:jest", | ||
| "outputs": ["{workspaceRoot}/coverage/apps/e2e/demo-e2e-resource-providers-e2e"], | ||
| "options": { | ||
| "jestConfig": "apps/e2e/demo-e2e-resource-providers/jest.e2e.config.ts", | ||
| "runInBand": true, | ||
| "passWithNoTests": true | ||
| } | ||
| } | ||
| } | ||
| } |
19 changes: 19 additions & 0 deletions
19
apps/e2e/demo-e2e-resource-providers/src/apps/main/index.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,19 @@ | ||
| import { App } from '@frontmcp/sdk'; | ||
| import { DataStoreService } from './providers/data-store.provider'; | ||
| import { CounterPlugin } from '../../plugins/counter/counter.plugin'; | ||
| import StoreSetTool from './tools/store-set.tool'; | ||
| import StoreGetTool from './tools/store-get.tool'; | ||
| import CounterIncrementTool from './tools/counter-increment.tool'; | ||
| import DebugProvidersTool from './tools/debug-providers.tool'; | ||
| import StoreContentsResource from './resources/store-contents.resource'; | ||
| import CounterStatusResource from './resources/counter-status.resource'; | ||
| import DebugProvidersResource from './resources/debug-providers.resource'; | ||
|
|
||
| @App({ | ||
| name: 'main', | ||
| providers: [DataStoreService], | ||
| plugins: [CounterPlugin], | ||
| tools: [StoreSetTool, StoreGetTool, CounterIncrementTool, DebugProvidersTool], | ||
| resources: [StoreContentsResource, CounterStatusResource, DebugProvidersResource], | ||
| }) | ||
| export class MainApp {} |
35 changes: 35 additions & 0 deletions
35
apps/e2e/demo-e2e-resource-providers/src/apps/main/providers/data-store.provider.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,35 @@ | ||
| import { Provider, ProviderScope } from '@frontmcp/sdk'; | ||
| import type { Token } from '@frontmcp/di'; | ||
|
|
||
| export const DATA_STORE_TOKEN: Token<DataStoreService> = Symbol('DataStore'); | ||
|
|
||
| export interface DataStoreEntry { | ||
| key: string; | ||
| value: string; | ||
| createdAt: number; | ||
| } | ||
|
|
||
| @Provider({ | ||
| name: 'DataStoreService', | ||
| scope: ProviderScope.GLOBAL, | ||
| }) | ||
| export class DataStoreService { | ||
| private readonly store = new Map<string, DataStoreEntry>(); | ||
| readonly instanceId = `store-${Math.random().toString(36).substring(2, 10)}`; | ||
|
|
||
| set(key: string, value: string): void { | ||
| this.store.set(key, { key, value, createdAt: Date.now() }); | ||
| } | ||
|
|
||
| get(key: string): DataStoreEntry | undefined { | ||
| return this.store.get(key); | ||
| } | ||
|
|
||
| getAll(): DataStoreEntry[] { | ||
| return Array.from(this.store.values()); | ||
| } | ||
|
|
||
| getInstanceId(): string { | ||
| return this.instanceId; | ||
| } | ||
| } |
23 changes: 23 additions & 0 deletions
23
apps/e2e/demo-e2e-resource-providers/src/apps/main/resources/counter-status.resource.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| import { Resource, ResourceContext } from '@frontmcp/sdk'; | ||
|
|
||
| /** | ||
| * Resource that accesses the CounterPlugin via context extension (this.counter). | ||
| * | ||
| * BUG UNDER TEST: Plugin context extensions should work in resources the same | ||
| * way they work in tools. If the plugin's exported provider is not in the | ||
| * resource's provider hierarchy, this.counter will throw | ||
| * ProviderNotRegisteredError. | ||
| */ | ||
| @Resource({ | ||
| uri: 'counter://status', | ||
| name: 'Counter Status', | ||
| description: 'Reads counter status via plugin context extension', | ||
| mimeType: 'application/json', | ||
| }) | ||
| export default class CounterStatusResource extends ResourceContext { | ||
| async execute() { | ||
| const count = this.counter.getCount(); | ||
| const instanceId = this.counter.getInstanceId(); | ||
| return { count, counterInstanceId: instanceId }; | ||
| } | ||
| } |
27 changes: 27 additions & 0 deletions
27
apps/e2e/demo-e2e-resource-providers/src/apps/main/resources/debug-providers.resource.ts
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,27 @@ | ||
| import { Resource, ResourceContext } from '@frontmcp/sdk'; | ||
| import { DataStoreService } from '../providers/data-store.provider'; | ||
|
|
||
| @Resource({ | ||
| uri: 'debug://providers', | ||
| name: 'Debug Providers', | ||
| description: 'Debug resource that reports provider resolution details', | ||
| mimeType: 'application/json', | ||
| }) | ||
| export default class DebugProvidersResource extends ResourceContext { | ||
| async execute() { | ||
| let storeInstanceId = 'NOT_RESOLVED'; | ||
| let error = ''; | ||
|
|
||
| try { | ||
| const store = this.get(DataStoreService); | ||
| storeInstanceId = store.getInstanceId(); | ||
| } catch (e: unknown) { | ||
| error = e instanceof Error ? `${e.constructor.name}: ${e.message}` : String(e); | ||
| } | ||
|
|
||
| return { | ||
| storeInstanceId, | ||
| error: error || undefined, | ||
| }; | ||
| } | ||
| } |
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.