Skip to content

Commit 89f4dcc

Browse files
committed
fix(pulse-fetch): fix MCP protocol compliance for embedded resources in saveAndReturn mode
- Wrap embedded resources in a 'resource' property to match MCP spec - Update both fresh scrapes and cached responses to use correct structure - Add comprehensive functional tests for resource shape validation - Update tool description examples to show correct format This fixes validation errors when using the MCP inspector with saveAndReturn mode.
1 parent 580c388 commit 89f4dcc

File tree

4 files changed

+181
-21
lines changed

4 files changed

+181
-21
lines changed

productionized/pulse-fetch/CHANGELOG.md

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,14 @@ and this project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0
77

88
## [Unreleased]
99

10+
### Fixed
11+
12+
- Fixed MCP protocol compliance issue with embedded resources in tool results
13+
- The `saveAndReturn` mode now properly wraps resource data in a `resource` property
14+
- Changed from `{ type: "resource", uri: "...", ... }` to `{ type: "resource", resource: { uri: "...", ... } }`
15+
- This resolves validation errors reported by the MCP inspector
16+
- Added comprehensive functional tests to verify proper resource shape validation
17+
1018
## [0.2.10] - 2025-07-03
1119

1220
### Changed

productionized/pulse-fetch/MANUAL_TESTING.md

Lines changed: 8 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -41,9 +41,9 @@ npm run test:manual:features # Features test suite
4141

4242
## Latest Test Results
4343

44-
**Test Date:** 2025-07-04 00:09 PT
45-
**Branch:** tadasant/bump-all-versions
46-
**Commit:** 35cd9db
44+
**Test Date:** 2025-07-03 19:06 PT
45+
**Branch:** tadasant/fix-invalid-union
46+
**Commit:** 580c388
4747
**Tested By:** Claude
4848
**Environment:** Local development with API keys from .env (FIRECRAWL_API_KEY, BRIGHTDATA_API_KEY, LLM_API_KEY)
4949

@@ -71,8 +71,8 @@ npm run test:manual:features # Features test suite
7171
**Details:**
7272

7373
- All tests passed with expected strategies
74-
- Strategy isolation working correctly after fixing parameter name (saveResult → resultHandling)
75-
- BrightData working after fixing environment variable name (BRIGHTDATA_BEARER_TOKEN → BRIGHTDATA_API_KEY)
74+
- Strategy isolation working correctly
75+
- All scraping services (Native, Firecrawl, BrightData) functioning properly
7676

7777
### Features Test Results
7878

@@ -86,12 +86,12 @@ npm run test:manual:features # Features test suite
8686
- Test framework correctly validated all credentials
8787
- ✅ scrape-tool.test.ts: All 3 tests passed
8888
- Basic scraping with automatic strategy selection working
89-
- Error handling working correctly (19s timeout test)
89+
- Error handling working correctly (22s timeout test)
9090
- Content extraction with Anthropic LLM successful
9191
- ✅ brightdata-scraping.test.ts: 1 test passed
92-
- BrightData client successfully scraped example.com (14s)
92+
- BrightData client successfully scraped example.com (2.7s)
9393
- ✅ firecrawl-scraping.test.ts: 1 test passed
94-
- Firecrawl client successfully scraped and converted to markdown (6s)
94+
- Firecrawl client successfully scraped and converted to markdown (1.9s)
9595
- ✅ native-scraping.test.ts: 2 tests passed
9696
- Native HTTP client working correctly
9797
- Successfully scraped example.com

productionized/pulse-fetch/shared/src/tools/scrape.ts

Lines changed: 26 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -154,9 +154,11 @@ saveAndReturn (embedded resource):
154154
"content": [
155155
{
156156
"type": "resource",
157-
"uri": "scraped://example.com/article_2024-01-15T10:30:00Z",
158-
"name": "https://example.com/article",
159-
"text": "Full article content..."
157+
"resource": {
158+
"uri": "scraped://example.com/article_2024-01-15T10:30:00Z",
159+
"name": "https://example.com/article",
160+
"text": "Full article content..."
161+
}
160162
}
161163
]
162164
}
@@ -314,11 +316,13 @@ The tool automatically:
314316
content: [
315317
{
316318
type: 'resource' as const,
317-
uri: cachedResource.uri,
318-
name: cachedResource.name,
319-
mimeType: cachedResource.mimeType,
320-
description: cachedResource.description,
321-
text: processedContent, // Original content without metadata
319+
resource: {
320+
uri: cachedResource.uri,
321+
name: cachedResource.name,
322+
mimeType: cachedResource.mimeType,
323+
description: cachedResource.description,
324+
text: processedContent, // Original content without metadata
325+
},
322326
},
323327
],
324328
};
@@ -451,6 +455,13 @@ The tool automatically:
451455
name?: string;
452456
mimeType?: string;
453457
description?: string;
458+
resource?: {
459+
uri: string;
460+
name?: string;
461+
mimeType?: string;
462+
description?: string;
463+
text?: string;
464+
};
454465
}>;
455466
} = {
456467
content: [],
@@ -516,11 +527,13 @@ The tool automatically:
516527
// For saveAndReturn, return embedded resource with content
517528
response.content.push({
518529
type: 'resource',
519-
uri: primaryUri!,
520-
name: url,
521-
mimeType: contentMimeType,
522-
description: resourceDescription,
523-
text: displayContent,
530+
resource: {
531+
uri: primaryUri!,
532+
name: url,
533+
mimeType: contentMimeType,
534+
description: resourceDescription,
535+
text: displayContent,
536+
},
524537
});
525538
}
526539
} catch (error) {
Lines changed: 139 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,139 @@
1+
import { describe, it, expect, vi, beforeEach } from 'vitest';
2+
import { CallToolResultSchema } from '@modelcontextprotocol/sdk/types.js';
3+
import { scrapeTool } from '../../shared/src/tools/scrape.js';
4+
import type { Server } from '@modelcontextprotocol/sdk/server/index.js';
5+
import type { IScrapingClients, StrategyConfigFactory } from '../../shared/src/server.js';
6+
import { ResourceStorageFactory } from '../../shared/src/storage/index.js';
7+
8+
// Mock dependencies
9+
vi.mock('../../shared/src/scraping-strategies.js', () => ({
10+
scrapeWithStrategy: vi.fn().mockResolvedValue({
11+
success: true,
12+
content: '<h1>Test Content</h1><p>This is test content.</p>',
13+
source: 'native',
14+
}),
15+
}));
16+
17+
vi.mock('../../shared/src/storage/index.js', () => ({
18+
ResourceStorageFactory: {
19+
create: vi.fn().mockResolvedValue({
20+
findByUrlAndExtract: vi.fn().mockResolvedValue([]),
21+
writeMulti: vi.fn().mockResolvedValue({
22+
raw: 'scraped://test.com/page_2024-01-01T00:00:00Z',
23+
cleaned: 'scraped://test.com/page_2024-01-01T00:00:00Z/cleaned',
24+
extracted: null,
25+
}),
26+
}),
27+
reset: vi.fn(),
28+
},
29+
}));
30+
31+
vi.mock('../../shared/src/extract/index.js', () => ({
32+
ExtractClientFactory: {
33+
isAvailable: vi.fn().mockReturnValue(false),
34+
createFromEnv: vi.fn().mockReturnValue(null),
35+
},
36+
}));
37+
38+
vi.mock('../../shared/src/clean/index.js', () => ({
39+
createCleaner: vi.fn().mockReturnValue({
40+
clean: vi.fn().mockResolvedValue('# Test Content\n\nThis is test content.'),
41+
}),
42+
}));
43+
44+
describe('Resource Shape Validation', () => {
45+
let mockServer: Server;
46+
let mockClientsFactory: () => IScrapingClients;
47+
let mockStrategyConfigFactory: StrategyConfigFactory;
48+
49+
beforeEach(() => {
50+
vi.clearAllMocks();
51+
ResourceStorageFactory.reset();
52+
53+
mockServer = {} as Server;
54+
mockClientsFactory = () => ({}) as IScrapingClients;
55+
mockStrategyConfigFactory = () => ({
56+
getAllDomainConfigs: vi.fn().mockReturnValue([]),
57+
getDomainConfig: vi.fn().mockReturnValue(null),
58+
});
59+
});
60+
61+
it('should return properly formatted embedded resource for saveAndReturn mode', async () => {
62+
const tool = scrapeTool(mockServer, mockClientsFactory, mockStrategyConfigFactory);
63+
64+
const result = await tool.handler({
65+
url: 'https://test.com/page',
66+
resultHandling: 'saveAndReturn',
67+
});
68+
69+
// Validate against MCP SDK schema
70+
const validation = CallToolResultSchema.safeParse(result);
71+
72+
if (!validation.success) {
73+
console.error('Validation error:', JSON.stringify(validation.error, null, 2));
74+
}
75+
76+
expect(validation.success).toBe(true);
77+
78+
// Check the specific structure
79+
expect(result.content).toHaveLength(1);
80+
expect(result.content[0].type).toBe('resource');
81+
82+
// The resource should be wrapped in a resource property
83+
expect(result.content[0]).toHaveProperty('resource');
84+
expect(result.content[0].resource).toMatchObject({
85+
uri: expect.stringContaining('scraped://'),
86+
name: 'https://test.com/page',
87+
mimeType: 'text/markdown',
88+
description: expect.stringContaining('Scraped content from'),
89+
text: expect.stringContaining('Test Content'),
90+
});
91+
});
92+
93+
it('should return properly formatted resource_link for saveOnly mode', async () => {
94+
const tool = scrapeTool(mockServer, mockClientsFactory, mockStrategyConfigFactory);
95+
96+
const result = await tool.handler({
97+
url: 'https://test.com/page',
98+
resultHandling: 'saveOnly',
99+
});
100+
101+
// Validate against MCP SDK schema
102+
const validation = CallToolResultSchema.safeParse(result);
103+
expect(validation.success).toBe(true);
104+
105+
// Check the specific structure
106+
expect(result.content).toHaveLength(1);
107+
expect(result.content[0].type).toBe('resource_link');
108+
expect(result.content[0]).not.toHaveProperty('resource');
109+
expect(result.content[0]).toMatchObject({
110+
type: 'resource_link',
111+
uri: expect.stringContaining('scraped://'),
112+
name: 'https://test.com/page',
113+
mimeType: 'text/markdown',
114+
description: expect.stringContaining('Scraped content from'),
115+
});
116+
});
117+
118+
it('should return properly formatted text for returnOnly mode', async () => {
119+
const tool = scrapeTool(mockServer, mockClientsFactory, mockStrategyConfigFactory);
120+
121+
const result = await tool.handler({
122+
url: 'https://test.com/page',
123+
resultHandling: 'returnOnly',
124+
});
125+
126+
// Validate against MCP SDK schema
127+
const validation = CallToolResultSchema.safeParse(result);
128+
expect(validation.success).toBe(true);
129+
130+
// Check the specific structure
131+
expect(result.content).toHaveLength(1);
132+
expect(result.content[0].type).toBe('text');
133+
expect(result.content[0]).toMatchObject({
134+
type: 'text',
135+
text: expect.stringContaining('Test Content'),
136+
});
137+
expect(result.content[0]).not.toHaveProperty('resource');
138+
});
139+
});

0 commit comments

Comments
 (0)