Skip to content

Tweak strict types setup, fix type errors under strictTypes.ts #806

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 1 commit into from
Jul 24, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion package.json
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
"fetch:spec-types": "curl -o spec.types.ts https://raw.githubusercontent.com/modelcontextprotocol/modelcontextprotocol/refs/heads/main/schema/draft/schema.ts",
"generate:strict-types": "tsx scripts/generateStrictTypes.ts",
"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)",
"test:strict-types": "node scripts/test-strict-types.js",
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this temp replaces the imports of "types.js" with "strictTypes.js" and runs all tests

"build": "npm run build:esm && npm run build:cjs",
"build:esm": "mkdir -p dist/esm && echo '{\"type\": \"module\"}' > dist/esm/package.json && tsc -p tsconfig.prod.json",
"build:esm:w": "npm run build:esm -- -w",
Expand All @@ -46,7 +47,7 @@
"examples:simple-server:w": "tsx --watch src/examples/server/simpleStreamableHttp.ts --oauth",
"prepack": "npm run build:esm && npm run build:cjs",
"lint": "eslint src/",
"test": "npm run fetch:spec-types && jest",
"test": "npm run fetch:spec-types && jest && npm run test:strict-types",
"start": "npm run server",
"server": "tsx watch --clear-screen=false src/cli.ts server",
"client": "tsx src/cli.ts client"
Expand Down
33 changes: 20 additions & 13 deletions scripts/generateStrictTypes.ts
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,10 @@ const strictTypesPath = join(__dirname, '../src/strictTypes.ts');

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

// Remove the @deprecated comment block
const deprecatedCommentPattern = /\/\*\*\s*\n\s*\*\s*@deprecated[\s\S]*?\*\/\s*\n/;
content = content.replace(deprecatedCommentPattern, '');
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this omits the "passthrough will be deprecated" comment that's at the top of types.js


// Add header comment
const header = `/**
* Types remove unknown
Expand All @@ -24,37 +28,40 @@ const header = `/**

`;

// Replace all .passthrough() with .strip()
content = content.replace(/\.passthrough\(\)/g, '.strip()');
// Replace all .passthrough() with a temporary marker
content = content.replace(/\.passthrough\(\)/g, '.__TEMP_MARKED_FOR_REMOVAL__()');

// Special handling for experimental and capabilities that should remain open
// These are explicitly designed to be extensible
const patternsToKeepOpen = [
// Keep experimental fields open as they're meant for extensions
/experimental: z\.optional\(z\.object\(\{\}\)\.strip\(\)\)/g,
/experimental: z\.optional\(z\.object\(\{\}\)\.__TEMP_MARKED_FOR_REMOVAL__\(\)\)/g,
// Keep _meta fields open as they're meant for arbitrary metadata
/_meta: z\.optional\(z\.object\(\{\}\)\.strip\(\)\)/g,
/_meta: z\.optional\(z\.object\(\{\}\)\.__TEMP_MARKED_FOR_REMOVAL__\(\)\)/g,
// Keep JSON Schema properties open as they can have arbitrary fields
/properties: z\.optional\(z\.object\(\{\}\)\.strip\(\)\)/g,
/properties: z\.optional\(z\.object\(\{\}\)\.__TEMP_MARKED_FOR_REMOVAL__\(\)\)/g,
// Keep BaseRequestParamsSchema passthrough for JSON-RPC param compatibility
/const BaseRequestParamsSchema = z\s*\n\s*\.object\([\s\S]*?\)\s*\n\s*\.strip\(\)/g,
/const BaseRequestParamsSchema = z\s*\n\s*\.object\([\s\S]*?\)\s*\n\s*\.__TEMP_MARKED_FOR_REMOVAL__\(\)/g,
// Keep BaseNotificationParamsSchema passthrough for JSON-RPC param compatibility
/const BaseNotificationParamsSchema = z\s*\n\s*\.object\([\s\S]*?\)\s*\n\s*\.strip\(\)/g,
/const BaseNotificationParamsSchema = z\s*\n\s*\.object\([\s\S]*?\)\s*\n\s*\.__TEMP_MARKED_FOR_REMOVAL__\(\)/g,
// Keep RequestMetaSchema passthrough for extensibility
/const RequestMetaSchema = z\s*\n\s*\.object\([\s\S]*?\)\s*\n\s*\.strip\(\)/g,
/const RequestMetaSchema = z\s*\n\s*\.object\([\s\S]*?\)\s*\n\s*\.__TEMP_MARKED_FOR_REMOVAL__\(\)/g,
// Keep structuredContent passthrough for tool-specific output
/structuredContent: z\.object\(\{\}\)\.strip\(\)\.optional\(\)/g,
/structuredContent: z\.object\(\{\}\)\.__TEMP_MARKED_FOR_REMOVAL__\(\)\.optional\(\)/g,
// Keep metadata passthrough for provider-specific data in sampling
/metadata: z\.optional\(z\.object\(\{\}\)\.strip\(\)\)/g,
/metadata: z\.optional\(z\.object\(\{\}\)\.__TEMP_MARKED_FOR_REMOVAL__\(\)\)/g,
];

// Revert strip back to passthrough for these special cases
// Revert marker back to passthrough for these special cases
patternsToKeepOpen.forEach(pattern => {
content = content.replace(pattern, (match) =>
match.replace('.strip()', '.passthrough()')
match.replace('.__TEMP_MARKED_FOR_REMOVAL__()', '.passthrough()')
);
});

// Remove the temporary marker from all remaining locations (these become no modifier)
content = content.replace(/\.__TEMP_MARKED_FOR_REMOVAL__\(\)/g, '');

// Add a comment explaining the difference
const explanation = `
/**
Expand All @@ -68,7 +75,7 @@ const explanation = `
* - structuredContent: Tool-specific output that can have arbitrary fields
* - metadata: Provider-specific metadata in sampling requests
*
* All other objects use .strip() to remove unknown properties while
* All other objects use default behavior (no modifier) to remove unknown properties while
* maintaining compatibility with extended protocols.
*/
`;
Expand Down
93 changes: 93 additions & 0 deletions scripts/test-strict-types.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,93 @@
#!/usr/bin/env node

import { execSync } from 'child_process';
import { readFileSync, writeFileSync, existsSync } from 'fs';
import { join, dirname } from 'path';
import { fileURLToPath } from 'url';

const __dirname = dirname(fileURLToPath(import.meta.url));
const rootDir = join(__dirname, '..');

const files = [
'src/client/index.ts',
'src/server/index.ts',
'src/server/mcp.ts'
];

console.log('Testing strict types compatibility...');
console.log('======================================\n');

// Backup original files
const backups = {};
files.forEach(file => {
const fullPath = join(rootDir, file);
if (existsSync(fullPath)) {
backups[file] = readFileSync(fullPath, 'utf8');

// Replace imports
const content = backups[file];
const newContent = content.replace(/from "\.\.\/types\.js"/g, 'from "../strictTypes.js"');
writeFileSync(fullPath, newContent);
console.log(`✓ Replaced imports in ${file}`);
}
});

console.log('\nRunning TypeScript compilation...\n');

try {
// Run TypeScript compilation
execSync('npm run build', { cwd: rootDir, stdio: 'pipe' });
console.log('✓ No type errors found!');
} catch (error) {
// Extract and format type errors
const output = error.stdout?.toString() || error.stderr?.toString() || '';
const lines = output.split('\n');

const errors = [];
let currentError = null;

lines.forEach((line, i) => {
if (line.includes('error TS')) {
if (currentError) {
errors.push(currentError);
}
currentError = {
file: line.split('(')[0],
location: line.match(/\((\d+),(\d+)\)/)?.[0] || '',
code: line.match(/error (TS\d+)/)?.[1] || '',
message: line.split(/error TS\d+:/)[1]?.trim() || '',
context: []
};
} else if (currentError && line.trim() && !line.startsWith('npm ')) {
currentError.context.push(line);
}
});

if (currentError) {
errors.push(currentError);
}

// Display errors
console.log(`Found ${errors.length} type error(s):\n`);

errors.forEach((error, index) => {
console.log(`${index + 1}. ${error.file}${error.location}`);
console.log(` Error ${error.code}: ${error.message}`);
if (error.context.length > 0) {
console.log(` Context:`);
error.context.slice(0, 3).forEach(line => {
console.log(` ${line.trim()}`);
});
}
console.log('');
});
}

// Restore original files
console.log('Restoring original files...');
Object.entries(backups).forEach(([file, content]) => {
const fullPath = join(rootDir, file);
writeFileSync(fullPath, content);
});

console.log('✓ Original files restored.');
35 changes: 35 additions & 0 deletions scripts/test-strict-types.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,35 @@
#!/bin/bash

# Script to test strict types by replacing imports and running tests

echo "Testing strict types compatibility..."
echo "======================================"

# Save original files
cp src/client/index.ts src/client/index.ts.bak
cp src/server/index.ts src/server/index.ts.bak
cp src/server/mcp.ts src/server/mcp.ts.bak

# Replace imports
sed -i '' 's/from "\.\.\/types\.js"/from "..\/strictTypes.js"/g' src/client/index.ts
sed -i '' 's/from "\.\.\/types\.js"/from "..\/strictTypes.js"/g' src/server/index.ts
sed -i '' 's/from "\.\.\/types\.js"/from "..\/strictTypes.js"/g' src/server/mcp.ts

echo "Replaced imports in:"
echo " - src/client/index.ts"
echo " - src/server/index.ts"
echo " - src/server/mcp.ts"
echo ""

# Run TypeScript compilation to get type errors
echo "Running TypeScript compilation..."
echo ""
npm run build 2>&1 | grep -E "(error TS|src/)" | grep -B1 "error TS" || true

# Restore original files
mv src/client/index.ts.bak src/client/index.ts
mv src/server/index.ts.bak src/server/index.ts
mv src/server/mcp.ts.bak src/server/mcp.ts

echo ""
echo "Original files restored."
46 changes: 37 additions & 9 deletions src/client/index.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -217,11 +217,11 @@ test("should connect new client to old, supported server version", async () => {
const [clientTransport, serverTransport] =
InMemoryTransport.createLinkedPair();

// Client initialization automatically uses LATEST_PROTOCOL_VERSION in the current SDK
const client = new Client(
{
name: "new client",
version: "1.0",
protocolVersion: LATEST_PROTOCOL_VERSION,
},
{
capabilities: {
Expand Down Expand Up @@ -287,7 +287,6 @@ test("should negotiate version when client is old, and newer server supports its
{
name: "old client",
version: "1.0",
protocolVersion: OLD_VERSION,
},
{
capabilities: {
Expand All @@ -297,6 +296,17 @@ test("should negotiate version when client is old, and newer server supports its
},
);

// Mock the request method to simulate an old client sending OLD_VERSION
const originalRequest = client.request.bind(client);
client.request = jest.fn(async (request, schema, options) => {
// If this is the initialize request, modify the protocol version to simulate old client
if (request.method === "initialize" && request.params) {
request.params.protocolVersion = OLD_VERSION;
}
// Call the original request method with the potentially modified request
return originalRequest(request, schema, options);
});

await Promise.all([
client.connect(clientTransport),
server.connect(serverTransport),
Expand Down Expand Up @@ -350,11 +360,13 @@ test("should throw when client is old, and server doesn't support its version",
const [clientTransport, serverTransport] =
InMemoryTransport.createLinkedPair();

// Client uses LATEST_PROTOCOL_VERSION by default, which is sufficient for this test
// The error occurs because the server returns FUTURE_VERSION (unsupported),
// not because of the client's version. Any client version would fail here.
const client = new Client(
{
name: "old client",
version: "1.0",
protocolVersion: OLD_VERSION,
},
{
capabilities: {
Expand Down Expand Up @@ -880,6 +892,7 @@ describe('outputSchema validation', () => {
server.setRequestHandler(CallToolRequestSchema, async (request) => {
if (request.params.name === 'test-tool') {
return {
content: [], // Required field for CallToolResult
structuredContent: { result: 'success', count: 42 },
};
}
Expand All @@ -903,7 +916,11 @@ describe('outputSchema validation', () => {

// Call the tool - should validate successfully
const result = await client.callTool({ name: 'test-tool' });
expect(result.structuredContent).toEqual({ result: 'success', count: 42 });
// Type narrowing: check if structuredContent exists before accessing
expect('structuredContent' in result).toBe(true);
if ('structuredContent' in result) {
expect(result.structuredContent).toEqual({ result: 'success', count: 42 });
}
});

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

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

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

// Call the tool - should validate successfully
const result = await client.callTool({ name: 'complex-tool' });
expect(result.structuredContent).toBeDefined();
const structuredContent = result.structuredContent as { name: string; age: number };
expect(structuredContent.name).toBe('John Doe');
expect(structuredContent.age).toBe(30);
// Type narrowing: check if structuredContent exists before accessing
expect('structuredContent' in result).toBe(true);
if ('structuredContent' in result) {
expect(result.structuredContent).toBeDefined();
const structuredContent = result.structuredContent as { name: string; age: number };
expect(structuredContent.name).toBe('John Doe');
expect(structuredContent.age).toBe(30);
}
});

/***
Expand Down Expand Up @@ -1269,6 +1296,7 @@ describe('outputSchema validation', () => {
if (request.params.name === 'strict-tool') {
// Return structured content with extra property
return {
content: [], // Required field for CallToolResult
structuredContent: {
name: 'John',
extraField: 'not allowed',
Expand Down
Loading