Skip to content

Commit a13c3bc

Browse files
committed
Tweak strict types setup, fix type errors under strictTypes.ts
- Modified strict type generation to just remove .passthrough() (.strict() is ~a no-op) - Fixed TypeScript errors when using strictTypes.ts (impl and tests) - Added a test pass that runs everything with strictTypes imported into src/client/index.ts, src/server/{index.ts,mcp.ts} - Added type annotations to Client/Server/McpServer methods for rollover ergo
1 parent 7b6060f commit a13c3bc

File tree

13 files changed

+297
-134
lines changed

13 files changed

+297
-134
lines changed

package.json

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -38,6 +38,7 @@
3838
"fetch:spec-types": "curl -o spec.types.ts https://raw.githubusercontent.com/modelcontextprotocol/modelcontextprotocol/refs/heads/main/schema/draft/schema.ts",
3939
"generate:strict-types": "tsx scripts/generateStrictTypes.ts",
4040
"check:strict-types": "npm run generate:strict-types && git diff --exit-code src/strictTypes.ts || (echo 'Error: strictTypes.ts is out of date. Run npm run generate:strict-types' && exit 1)",
41+
"test:strict-types": "node scripts/test-strict-types.js",
4142
"build": "npm run build:esm && npm run build:cjs",
4243
"build:esm": "mkdir -p dist/esm && echo '{\"type\": \"module\"}' > dist/esm/package.json && tsc -p tsconfig.prod.json",
4344
"build:esm:w": "npm run build:esm -- -w",
@@ -46,7 +47,7 @@
4647
"examples:simple-server:w": "tsx --watch src/examples/server/simpleStreamableHttp.ts --oauth",
4748
"prepack": "npm run build:esm && npm run build:cjs",
4849
"lint": "eslint src/",
49-
"test": "npm run fetch:spec-types && jest",
50+
"test": "npm run fetch:spec-types && jest && npm run test:strict-types",
5051
"start": "npm run server",
5152
"server": "tsx watch --clear-screen=false src/cli.ts server",
5253
"client": "tsx src/cli.ts client"

scripts/generateStrictTypes.ts

Lines changed: 20 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,10 @@ const strictTypesPath = join(__dirname, '../src/strictTypes.ts');
1111

1212
let content = readFileSync(typesPath, 'utf-8');
1313

14+
// Remove the @deprecated comment block
15+
const deprecatedCommentPattern = /\/\*\*\s*\n\s*\*\s*@deprecated[\s\S]*?\*\/\s*\n/;
16+
content = content.replace(deprecatedCommentPattern, '');
17+
1418
// Add header comment
1519
const header = `/**
1620
* Types remove unknown
@@ -24,37 +28,40 @@ const header = `/**
2428
2529
`;
2630

27-
// Replace all .passthrough() with .strip()
28-
content = content.replace(/\.passthrough\(\)/g, '.strip()');
31+
// Replace all .passthrough() with a temporary marker
32+
content = content.replace(/\.passthrough\(\)/g, '.__TEMP_MARKED_FOR_REMOVAL__()');
2933

3034
// Special handling for experimental and capabilities that should remain open
3135
// These are explicitly designed to be extensible
3236
const patternsToKeepOpen = [
3337
// Keep experimental fields open as they're meant for extensions
34-
/experimental: z\.optional\(z\.object\(\{\}\)\.strip\(\)\)/g,
38+
/experimental: z\.optional\(z\.object\(\{\}\)\.__TEMP_MARKED_FOR_REMOVAL__\(\)\)/g,
3539
// Keep _meta fields open as they're meant for arbitrary metadata
36-
/_meta: z\.optional\(z\.object\(\{\}\)\.strip\(\)\)/g,
40+
/_meta: z\.optional\(z\.object\(\{\}\)\.__TEMP_MARKED_FOR_REMOVAL__\(\)\)/g,
3741
// Keep JSON Schema properties open as they can have arbitrary fields
38-
/properties: z\.optional\(z\.object\(\{\}\)\.strip\(\)\)/g,
42+
/properties: z\.optional\(z\.object\(\{\}\)\.__TEMP_MARKED_FOR_REMOVAL__\(\)\)/g,
3943
// Keep BaseRequestParamsSchema passthrough for JSON-RPC param compatibility
40-
/const BaseRequestParamsSchema = z\s*\n\s*\.object\([\s\S]*?\)\s*\n\s*\.strip\(\)/g,
44+
/const BaseRequestParamsSchema = z\s*\n\s*\.object\([\s\S]*?\)\s*\n\s*\.__TEMP_MARKED_FOR_REMOVAL__\(\)/g,
4145
// Keep BaseNotificationParamsSchema passthrough for JSON-RPC param compatibility
42-
/const BaseNotificationParamsSchema = z\s*\n\s*\.object\([\s\S]*?\)\s*\n\s*\.strip\(\)/g,
46+
/const BaseNotificationParamsSchema = z\s*\n\s*\.object\([\s\S]*?\)\s*\n\s*\.__TEMP_MARKED_FOR_REMOVAL__\(\)/g,
4347
// Keep RequestMetaSchema passthrough for extensibility
44-
/const RequestMetaSchema = z\s*\n\s*\.object\([\s\S]*?\)\s*\n\s*\.strip\(\)/g,
48+
/const RequestMetaSchema = z\s*\n\s*\.object\([\s\S]*?\)\s*\n\s*\.__TEMP_MARKED_FOR_REMOVAL__\(\)/g,
4549
// Keep structuredContent passthrough for tool-specific output
46-
/structuredContent: z\.object\(\{\}\)\.strip\(\)\.optional\(\)/g,
50+
/structuredContent: z\.object\(\{\}\)\.__TEMP_MARKED_FOR_REMOVAL__\(\)\.optional\(\)/g,
4751
// Keep metadata passthrough for provider-specific data in sampling
48-
/metadata: z\.optional\(z\.object\(\{\}\)\.strip\(\)\)/g,
52+
/metadata: z\.optional\(z\.object\(\{\}\)\.__TEMP_MARKED_FOR_REMOVAL__\(\)\)/g,
4953
];
5054

51-
// Revert strip back to passthrough for these special cases
55+
// Revert marker back to passthrough for these special cases
5256
patternsToKeepOpen.forEach(pattern => {
5357
content = content.replace(pattern, (match) =>
54-
match.replace('.strip()', '.passthrough()')
58+
match.replace('.__TEMP_MARKED_FOR_REMOVAL__()', '.passthrough()')
5559
);
5660
});
5761

62+
// Remove the temporary marker from all remaining locations (these become no modifier)
63+
content = content.replace(/\.__TEMP_MARKED_FOR_REMOVAL__\(\)/g, '');
64+
5865
// Add a comment explaining the difference
5966
const explanation = `
6067
/**
@@ -68,7 +75,7 @@ const explanation = `
6875
* - structuredContent: Tool-specific output that can have arbitrary fields
6976
* - metadata: Provider-specific metadata in sampling requests
7077
*
71-
* All other objects use .strip() to remove unknown properties while
78+
* All other objects use default behavior (no modifier) to remove unknown properties while
7279
* maintaining compatibility with extended protocols.
7380
*/
7481
`;

scripts/test-strict-types.js

Lines changed: 93 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,93 @@
1+
#!/usr/bin/env node
2+
3+
import { execSync } from 'child_process';
4+
import { readFileSync, writeFileSync, existsSync } from 'fs';
5+
import { join, dirname } from 'path';
6+
import { fileURLToPath } from 'url';
7+
8+
const __dirname = dirname(fileURLToPath(import.meta.url));
9+
const rootDir = join(__dirname, '..');
10+
11+
const files = [
12+
'src/client/index.ts',
13+
'src/server/index.ts',
14+
'src/server/mcp.ts'
15+
];
16+
17+
console.log('Testing strict types compatibility...');
18+
console.log('======================================\n');
19+
20+
// Backup original files
21+
const backups = {};
22+
files.forEach(file => {
23+
const fullPath = join(rootDir, file);
24+
if (existsSync(fullPath)) {
25+
backups[file] = readFileSync(fullPath, 'utf8');
26+
27+
// Replace imports
28+
const content = backups[file];
29+
const newContent = content.replace(/from "\.\.\/types\.js"/g, 'from "../strictTypes.js"');
30+
writeFileSync(fullPath, newContent);
31+
console.log(`✓ Replaced imports in ${file}`);
32+
}
33+
});
34+
35+
console.log('\nRunning TypeScript compilation...\n');
36+
37+
try {
38+
// Run TypeScript compilation
39+
execSync('npm run build', { cwd: rootDir, stdio: 'pipe' });
40+
console.log('✓ No type errors found!');
41+
} catch (error) {
42+
// Extract and format type errors
43+
const output = error.stdout?.toString() || error.stderr?.toString() || '';
44+
const lines = output.split('\n');
45+
46+
const errors = [];
47+
let currentError = null;
48+
49+
lines.forEach((line, i) => {
50+
if (line.includes('error TS')) {
51+
if (currentError) {
52+
errors.push(currentError);
53+
}
54+
currentError = {
55+
file: line.split('(')[0],
56+
location: line.match(/\((\d+),(\d+)\)/)?.[0] || '',
57+
code: line.match(/error (TS\d+)/)?.[1] || '',
58+
message: line.split(/error TS\d+:/)[1]?.trim() || '',
59+
context: []
60+
};
61+
} else if (currentError && line.trim() && !line.startsWith('npm ')) {
62+
currentError.context.push(line);
63+
}
64+
});
65+
66+
if (currentError) {
67+
errors.push(currentError);
68+
}
69+
70+
// Display errors
71+
console.log(`Found ${errors.length} type error(s):\n`);
72+
73+
errors.forEach((error, index) => {
74+
console.log(`${index + 1}. ${error.file}${error.location}`);
75+
console.log(` Error ${error.code}: ${error.message}`);
76+
if (error.context.length > 0) {
77+
console.log(` Context:`);
78+
error.context.slice(0, 3).forEach(line => {
79+
console.log(` ${line.trim()}`);
80+
});
81+
}
82+
console.log('');
83+
});
84+
}
85+
86+
// Restore original files
87+
console.log('Restoring original files...');
88+
Object.entries(backups).forEach(([file, content]) => {
89+
const fullPath = join(rootDir, file);
90+
writeFileSync(fullPath, content);
91+
});
92+
93+
console.log('✓ Original files restored.');

scripts/test-strict-types.sh

Lines changed: 35 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,35 @@
1+
#!/bin/bash
2+
3+
# Script to test strict types by replacing imports and running tests
4+
5+
echo "Testing strict types compatibility..."
6+
echo "======================================"
7+
8+
# Save original files
9+
cp src/client/index.ts src/client/index.ts.bak
10+
cp src/server/index.ts src/server/index.ts.bak
11+
cp src/server/mcp.ts src/server/mcp.ts.bak
12+
13+
# Replace imports
14+
sed -i '' 's/from "\.\.\/types\.js"/from "..\/strictTypes.js"/g' src/client/index.ts
15+
sed -i '' 's/from "\.\.\/types\.js"/from "..\/strictTypes.js"/g' src/server/index.ts
16+
sed -i '' 's/from "\.\.\/types\.js"/from "..\/strictTypes.js"/g' src/server/mcp.ts
17+
18+
echo "Replaced imports in:"
19+
echo " - src/client/index.ts"
20+
echo " - src/server/index.ts"
21+
echo " - src/server/mcp.ts"
22+
echo ""
23+
24+
# Run TypeScript compilation to get type errors
25+
echo "Running TypeScript compilation..."
26+
echo ""
27+
npm run build 2>&1 | grep -E "(error TS|src/)" | grep -B1 "error TS" || true
28+
29+
# Restore original files
30+
mv src/client/index.ts.bak src/client/index.ts
31+
mv src/server/index.ts.bak src/server/index.ts
32+
mv src/server/mcp.ts.bak src/server/mcp.ts
33+
34+
echo ""
35+
echo "Original files restored."

src/client/index.test.ts

Lines changed: 37 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -217,11 +217,11 @@ test("should connect new client to old, supported server version", async () => {
217217
const [clientTransport, serverTransport] =
218218
InMemoryTransport.createLinkedPair();
219219

220+
// Client initialization automatically uses LATEST_PROTOCOL_VERSION in the current SDK
220221
const client = new Client(
221222
{
222223
name: "new client",
223224
version: "1.0",
224-
protocolVersion: LATEST_PROTOCOL_VERSION,
225225
},
226226
{
227227
capabilities: {
@@ -287,7 +287,6 @@ test("should negotiate version when client is old, and newer server supports its
287287
{
288288
name: "old client",
289289
version: "1.0",
290-
protocolVersion: OLD_VERSION,
291290
},
292291
{
293292
capabilities: {
@@ -297,6 +296,17 @@ test("should negotiate version when client is old, and newer server supports its
297296
},
298297
);
299298

299+
// Mock the request method to simulate an old client sending OLD_VERSION
300+
const originalRequest = client.request.bind(client);
301+
client.request = jest.fn(async (request, schema, options) => {
302+
// If this is the initialize request, modify the protocol version to simulate old client
303+
if (request.method === "initialize" && request.params) {
304+
request.params.protocolVersion = OLD_VERSION;
305+
}
306+
// Call the original request method with the potentially modified request
307+
return originalRequest(request, schema, options);
308+
});
309+
300310
await Promise.all([
301311
client.connect(clientTransport),
302312
server.connect(serverTransport),
@@ -350,11 +360,13 @@ test("should throw when client is old, and server doesn't support its version",
350360
const [clientTransport, serverTransport] =
351361
InMemoryTransport.createLinkedPair();
352362

363+
// Client uses LATEST_PROTOCOL_VERSION by default, which is sufficient for this test
364+
// The error occurs because the server returns FUTURE_VERSION (unsupported),
365+
// not because of the client's version. Any client version would fail here.
353366
const client = new Client(
354367
{
355368
name: "old client",
356369
version: "1.0",
357-
protocolVersion: OLD_VERSION,
358370
},
359371
{
360372
capabilities: {
@@ -880,6 +892,7 @@ describe('outputSchema validation', () => {
880892
server.setRequestHandler(CallToolRequestSchema, async (request) => {
881893
if (request.params.name === 'test-tool') {
882894
return {
895+
content: [], // Required field for CallToolResult
883896
structuredContent: { result: 'success', count: 42 },
884897
};
885898
}
@@ -903,7 +916,11 @@ describe('outputSchema validation', () => {
903916

904917
// Call the tool - should validate successfully
905918
const result = await client.callTool({ name: 'test-tool' });
906-
expect(result.structuredContent).toEqual({ result: 'success', count: 42 });
919+
// Type narrowing: check if structuredContent exists before accessing
920+
expect('structuredContent' in result).toBe(true);
921+
if ('structuredContent' in result) {
922+
expect(result.structuredContent).toEqual({ result: 'success', count: 42 });
923+
}
907924
});
908925

909926
/***
@@ -955,6 +972,7 @@ describe('outputSchema validation', () => {
955972
if (request.params.name === 'test-tool') {
956973
// Return invalid structured content (count is string instead of number)
957974
return {
975+
content: [], // Required field for CallToolResult
958976
structuredContent: { result: 'success', count: 'not a number' },
959977
};
960978
}
@@ -1120,7 +1138,11 @@ describe('outputSchema validation', () => {
11201138

11211139
// Call the tool - should work normally without validation
11221140
const result = await client.callTool({ name: 'test-tool' });
1123-
expect(result.content).toEqual([{ type: 'text', text: 'Normal response' }]);
1141+
// Type narrowing: check if content exists before accessing
1142+
expect('content' in result).toBe(true);
1143+
if ('content' in result) {
1144+
expect(result.content).toEqual([{ type: 'text', text: 'Normal response' }]);
1145+
}
11241146
});
11251147

11261148
/***
@@ -1184,6 +1206,7 @@ describe('outputSchema validation', () => {
11841206
server.setRequestHandler(CallToolRequestSchema, async (request) => {
11851207
if (request.params.name === 'complex-tool') {
11861208
return {
1209+
content: [], // Required field for CallToolResult
11871210
structuredContent: {
11881211
name: 'John Doe',
11891212
age: 30,
@@ -1215,10 +1238,14 @@ describe('outputSchema validation', () => {
12151238

12161239
// Call the tool - should validate successfully
12171240
const result = await client.callTool({ name: 'complex-tool' });
1218-
expect(result.structuredContent).toBeDefined();
1219-
const structuredContent = result.structuredContent as { name: string; age: number };
1220-
expect(structuredContent.name).toBe('John Doe');
1221-
expect(structuredContent.age).toBe(30);
1241+
// Type narrowing: check if structuredContent exists before accessing
1242+
expect('structuredContent' in result).toBe(true);
1243+
if ('structuredContent' in result) {
1244+
expect(result.structuredContent).toBeDefined();
1245+
const structuredContent = result.structuredContent as { name: string; age: number };
1246+
expect(structuredContent.name).toBe('John Doe');
1247+
expect(structuredContent.age).toBe(30);
1248+
}
12221249
});
12231250

12241251
/***
@@ -1269,6 +1296,7 @@ describe('outputSchema validation', () => {
12691296
if (request.params.name === 'strict-tool') {
12701297
// Return structured content with extra property
12711298
return {
1299+
content: [], // Required field for CallToolResult
12721300
structuredContent: {
12731301
name: 'John',
12741302
extraField: 'not allowed',

0 commit comments

Comments
 (0)