mirror of
https://github.com/eyaltoledano/claude-task-master.git
synced 2026-01-30 06:12:05 +00:00
fix: resolve build issues and add missing tests
- Move validateMcpMetadata to @tm/mcp shared utils (TypeScript) - Remove duplicate implementation from mcp-server/src/tools/utils.js - Add comprehensive unit tests for validateMcpMetadata function - Fix dependencies type in metadata-preservation test (number → string) - Fix TypeScript union type handling in test assertions
This commit is contained in:
@@ -442,3 +442,63 @@ export function withToolContext<TArgs extends { projectRoot?: string }>(
|
|||||||
}
|
}
|
||||||
);
|
);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Validates and parses metadata string for MCP tools.
|
||||||
|
* Checks environment flag, validates JSON format, and ensures metadata is a plain object.
|
||||||
|
*
|
||||||
|
* @param metadataString - JSON string to parse and validate
|
||||||
|
* @param errorResponseFn - Function to create error response
|
||||||
|
* @returns Object with parsed metadata or error
|
||||||
|
*/
|
||||||
|
export function validateMcpMetadata(
|
||||||
|
metadataString: string | null | undefined,
|
||||||
|
errorResponseFn: (message: string) => ContentResult
|
||||||
|
): { parsedMetadata: Record<string, unknown> | null; error?: ContentResult } {
|
||||||
|
// Return null if no metadata provided
|
||||||
|
if (!metadataString) {
|
||||||
|
return { parsedMetadata: null };
|
||||||
|
}
|
||||||
|
|
||||||
|
// Check if metadata updates are allowed via environment variable
|
||||||
|
const allowMetadataUpdates =
|
||||||
|
process.env.TASK_MASTER_ALLOW_METADATA_UPDATES === 'true';
|
||||||
|
if (!allowMetadataUpdates) {
|
||||||
|
return {
|
||||||
|
parsedMetadata: null,
|
||||||
|
error: errorResponseFn(
|
||||||
|
'Metadata updates are disabled. Set TASK_MASTER_ALLOW_METADATA_UPDATES=true in your MCP server environment to enable metadata modifications.'
|
||||||
|
)
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
// Parse and validate JSON
|
||||||
|
try {
|
||||||
|
const parsedMetadata = JSON.parse(metadataString);
|
||||||
|
|
||||||
|
// Ensure it's a plain object (not null, not array)
|
||||||
|
if (
|
||||||
|
typeof parsedMetadata !== 'object' ||
|
||||||
|
parsedMetadata === null ||
|
||||||
|
Array.isArray(parsedMetadata)
|
||||||
|
) {
|
||||||
|
return {
|
||||||
|
parsedMetadata: null,
|
||||||
|
error: errorResponseFn(
|
||||||
|
'Invalid metadata: must be a JSON object (not null or array)'
|
||||||
|
)
|
||||||
|
};
|
||||||
|
}
|
||||||
|
|
||||||
|
return { parsedMetadata };
|
||||||
|
} catch (parseError: unknown) {
|
||||||
|
const message =
|
||||||
|
parseError instanceof Error ? parseError.message : 'Unknown parse error';
|
||||||
|
return {
|
||||||
|
parsedMetadata: null,
|
||||||
|
error: errorResponseFn(
|
||||||
|
`Invalid metadata JSON: ${message}. Provide a valid JSON object string.`
|
||||||
|
)
|
||||||
|
};
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|||||||
@@ -829,72 +829,6 @@ function checkProgressCapability(reportProgress, log) {
|
|||||||
return reportProgress;
|
return reportProgress;
|
||||||
}
|
}
|
||||||
|
|
||||||
/**
|
|
||||||
* Validates and parses metadata string for MCP tools.
|
|
||||||
* Checks environment flag, validates JSON format, and ensures metadata is a plain object.
|
|
||||||
*
|
|
||||||
* @param {string|null|undefined} metadataString - JSON string to parse and validate
|
|
||||||
* @param {Function} createErrorResponse - Function to create error response
|
|
||||||
* @returns {{parsedMetadata: Object|null, error?: Object}} Object with parsed metadata or error
|
|
||||||
*
|
|
||||||
* @example Success case:
|
|
||||||
* const result = validateMcpMetadata('{"key":"value"}', createErrorResponse);
|
|
||||||
* if (result.error) return result.error;
|
|
||||||
* const metadata = result.parsedMetadata; // { key: "value" }
|
|
||||||
*
|
|
||||||
* @example Disabled case:
|
|
||||||
* // When TASK_MASTER_ALLOW_METADATA_UPDATES !== 'true'
|
|
||||||
* const result = validateMcpMetadata('{"key":"value"}', createErrorResponse);
|
|
||||||
* // Returns: { error: <error response> }
|
|
||||||
*
|
|
||||||
* @example Invalid JSON:
|
|
||||||
* const result = validateMcpMetadata('{invalid}', createErrorResponse);
|
|
||||||
* // Returns: { error: <error response> }
|
|
||||||
*/
|
|
||||||
export function validateMcpMetadata(metadataString, createErrorResponse) {
|
|
||||||
// Return null if no metadata provided
|
|
||||||
if (!metadataString) {
|
|
||||||
return { parsedMetadata: null };
|
|
||||||
}
|
|
||||||
|
|
||||||
// Check if metadata updates are allowed via environment variable
|
|
||||||
const allowMetadataUpdates =
|
|
||||||
process.env.TASK_MASTER_ALLOW_METADATA_UPDATES === 'true';
|
|
||||||
if (!allowMetadataUpdates) {
|
|
||||||
return {
|
|
||||||
error: createErrorResponse(
|
|
||||||
'Metadata updates are disabled. Set TASK_MASTER_ALLOW_METADATA_UPDATES=true in your MCP server environment to enable metadata modifications.'
|
|
||||||
)
|
|
||||||
};
|
|
||||||
}
|
|
||||||
|
|
||||||
// Parse and validate JSON
|
|
||||||
try {
|
|
||||||
const parsedMetadata = JSON.parse(metadataString);
|
|
||||||
|
|
||||||
// Ensure it's a plain object (not null, not array)
|
|
||||||
if (
|
|
||||||
typeof parsedMetadata !== 'object' ||
|
|
||||||
parsedMetadata === null ||
|
|
||||||
Array.isArray(parsedMetadata)
|
|
||||||
) {
|
|
||||||
return {
|
|
||||||
error: createErrorResponse(
|
|
||||||
'Invalid metadata: must be a JSON object (not null or array)'
|
|
||||||
)
|
|
||||||
};
|
|
||||||
}
|
|
||||||
|
|
||||||
return { parsedMetadata };
|
|
||||||
} catch (parseError) {
|
|
||||||
return {
|
|
||||||
error: createErrorResponse(
|
|
||||||
`Invalid metadata JSON: ${parseError.message}. Provide a valid JSON object string.`
|
|
||||||
)
|
|
||||||
};
|
|
||||||
}
|
|
||||||
}
|
|
||||||
|
|
||||||
// Ensure all functions are exported
|
// Ensure all functions are exported
|
||||||
export {
|
export {
|
||||||
getProjectRoot,
|
getProjectRoot,
|
||||||
|
|||||||
@@ -218,7 +218,7 @@ describe('AI Operation Metadata Preservation - Integration Tests', () => {
|
|||||||
description: 'Second step generated by AI',
|
description: 'Second step generated by AI',
|
||||||
status: 'pending',
|
status: 'pending',
|
||||||
priority: 'medium',
|
priority: 'medium',
|
||||||
dependencies: [1],
|
dependencies: ['1'],
|
||||||
details: 'More details',
|
details: 'More details',
|
||||||
testStrategy: 'More tests'
|
testStrategy: 'More tests'
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -14,10 +14,11 @@
|
|||||||
* - Direct function integration (covered by the validation tests here)
|
* - Direct function integration (covered by the validation tests here)
|
||||||
*/
|
*/
|
||||||
|
|
||||||
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';
|
import { afterEach, beforeEach, describe, expect, it } from 'vitest';
|
||||||
import * as fs from 'fs';
|
import * as fs from 'fs';
|
||||||
import * as path from 'path';
|
import * as path from 'path';
|
||||||
import * as os from 'os';
|
import * as os from 'os';
|
||||||
|
import { validateMcpMetadata } from '@tm/mcp';
|
||||||
|
|
||||||
describe('MCP Tool Metadata Updates - Integration Tests', () => {
|
describe('MCP Tool Metadata Updates - Integration Tests', () => {
|
||||||
let tempDir: string;
|
let tempDir: string;
|
||||||
@@ -198,7 +199,7 @@ describe('MCP Tool Metadata Updates - Integration Tests', () => {
|
|||||||
|
|
||||||
describe('metadata-only update detection', () => {
|
describe('metadata-only update detection', () => {
|
||||||
it('should detect metadata-only update when prompt is empty', () => {
|
it('should detect metadata-only update when prompt is empty', () => {
|
||||||
const prompt = '';
|
const prompt: string = '';
|
||||||
const metadata = { key: 'value' };
|
const metadata = { key: 'value' };
|
||||||
|
|
||||||
const isMetadataOnly = metadata && (!prompt || prompt.trim() === '');
|
const isMetadataOnly = metadata && (!prompt || prompt.trim() === '');
|
||||||
@@ -206,7 +207,7 @@ describe('MCP Tool Metadata Updates - Integration Tests', () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it('should detect metadata-only update when prompt is whitespace', () => {
|
it('should detect metadata-only update when prompt is whitespace', () => {
|
||||||
const prompt = ' ';
|
const prompt: string = ' ';
|
||||||
const metadata = { key: 'value' };
|
const metadata = { key: 'value' };
|
||||||
|
|
||||||
const isMetadataOnly = metadata && (!prompt || prompt.trim() === '');
|
const isMetadataOnly = metadata && (!prompt || prompt.trim() === '');
|
||||||
@@ -214,7 +215,7 @@ describe('MCP Tool Metadata Updates - Integration Tests', () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it('should not be metadata-only when prompt is provided', () => {
|
it('should not be metadata-only when prompt is provided', () => {
|
||||||
const prompt = 'Update task details';
|
const prompt: string = 'Update task details';
|
||||||
const metadata = { key: 'value' };
|
const metadata = { key: 'value' };
|
||||||
|
|
||||||
const isMetadataOnly = metadata && (!prompt || prompt.trim() === '');
|
const isMetadataOnly = metadata && (!prompt || prompt.trim() === '');
|
||||||
@@ -222,7 +223,7 @@ describe('MCP Tool Metadata Updates - Integration Tests', () => {
|
|||||||
});
|
});
|
||||||
|
|
||||||
it('should not be metadata-only when neither is provided', () => {
|
it('should not be metadata-only when neither is provided', () => {
|
||||||
const prompt = '';
|
const prompt: string = '';
|
||||||
const metadata = null;
|
const metadata = null;
|
||||||
|
|
||||||
const isMetadataOnly = metadata && (!prompt || prompt.trim() === '');
|
const isMetadataOnly = metadata && (!prompt || prompt.trim() === '');
|
||||||
@@ -353,3 +354,187 @@ describe('MCP Tool Metadata Updates - Integration Tests', () => {
|
|||||||
});
|
});
|
||||||
});
|
});
|
||||||
});
|
});
|
||||||
|
|
||||||
|
/**
|
||||||
|
* Unit tests for the actual validateMcpMetadata function from @tm/mcp
|
||||||
|
* These tests verify the security gate behavior for MCP metadata updates.
|
||||||
|
*/
|
||||||
|
describe('validateMcpMetadata function', () => {
|
||||||
|
// Mock error response creator that matches the MCP ContentResult format
|
||||||
|
const mockCreateErrorResponse = (message: string) => ({
|
||||||
|
content: [{ type: 'text' as const, text: `Error: ${message}` }],
|
||||||
|
isError: true
|
||||||
|
});
|
||||||
|
|
||||||
|
// Helper to safely extract text from content
|
||||||
|
const getErrorText = (
|
||||||
|
error: { content: Array<{ type: string; text?: string }> } | undefined
|
||||||
|
): string => {
|
||||||
|
if (!error?.content?.[0]) return '';
|
||||||
|
const content = error.content[0];
|
||||||
|
return 'text' in content ? (content.text ?? '') : '';
|
||||||
|
};
|
||||||
|
|
||||||
|
afterEach(() => {
|
||||||
|
delete process.env.TASK_MASTER_ALLOW_METADATA_UPDATES;
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('when metadataString is null/undefined', () => {
|
||||||
|
it('should return null parsedMetadata for undefined input', () => {
|
||||||
|
const result = validateMcpMetadata(undefined, mockCreateErrorResponse);
|
||||||
|
expect(result.parsedMetadata).toBeNull();
|
||||||
|
expect(result.error).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should return null parsedMetadata for null input', () => {
|
||||||
|
const result = validateMcpMetadata(null, mockCreateErrorResponse);
|
||||||
|
expect(result.parsedMetadata).toBeNull();
|
||||||
|
expect(result.error).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should return null parsedMetadata for empty string', () => {
|
||||||
|
const result = validateMcpMetadata('', mockCreateErrorResponse);
|
||||||
|
expect(result.parsedMetadata).toBeNull();
|
||||||
|
expect(result.error).toBeUndefined();
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('when TASK_MASTER_ALLOW_METADATA_UPDATES is not set', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
delete process.env.TASK_MASTER_ALLOW_METADATA_UPDATES;
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should return error when flag is not set', () => {
|
||||||
|
const result = validateMcpMetadata(
|
||||||
|
'{"key": "value"}',
|
||||||
|
mockCreateErrorResponse
|
||||||
|
);
|
||||||
|
expect(result.error).toBeDefined();
|
||||||
|
expect(result.error?.isError).toBe(true);
|
||||||
|
expect(getErrorText(result.error)).toContain(
|
||||||
|
'TASK_MASTER_ALLOW_METADATA_UPDATES'
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should return error when flag is set to "false"', () => {
|
||||||
|
process.env.TASK_MASTER_ALLOW_METADATA_UPDATES = 'false';
|
||||||
|
const result = validateMcpMetadata(
|
||||||
|
'{"key": "value"}',
|
||||||
|
mockCreateErrorResponse
|
||||||
|
);
|
||||||
|
expect(result.error).toBeDefined();
|
||||||
|
expect(result.error?.isError).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should return error when flag is "TRUE" (case sensitive)', () => {
|
||||||
|
process.env.TASK_MASTER_ALLOW_METADATA_UPDATES = 'TRUE';
|
||||||
|
const result = validateMcpMetadata(
|
||||||
|
'{"key": "value"}',
|
||||||
|
mockCreateErrorResponse
|
||||||
|
);
|
||||||
|
expect(result.error).toBeDefined();
|
||||||
|
expect(result.error?.isError).toBe(true);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should return error when flag is "True" (case sensitive)', () => {
|
||||||
|
process.env.TASK_MASTER_ALLOW_METADATA_UPDATES = 'True';
|
||||||
|
const result = validateMcpMetadata(
|
||||||
|
'{"key": "value"}',
|
||||||
|
mockCreateErrorResponse
|
||||||
|
);
|
||||||
|
expect(result.error).toBeDefined();
|
||||||
|
expect(result.error?.isError).toBe(true);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|
||||||
|
describe('when TASK_MASTER_ALLOW_METADATA_UPDATES is "true"', () => {
|
||||||
|
beforeEach(() => {
|
||||||
|
process.env.TASK_MASTER_ALLOW_METADATA_UPDATES = 'true';
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should return parsed metadata for valid JSON object', () => {
|
||||||
|
const result = validateMcpMetadata(
|
||||||
|
'{"key": "value"}',
|
||||||
|
mockCreateErrorResponse
|
||||||
|
);
|
||||||
|
expect(result.parsedMetadata).toEqual({ key: 'value' });
|
||||||
|
expect(result.error).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should return parsed metadata for complex nested object', () => {
|
||||||
|
const complexMeta = {
|
||||||
|
githubIssue: 42,
|
||||||
|
sprint: 'Q1-S3',
|
||||||
|
nested: { deep: { value: true } },
|
||||||
|
array: [1, 2, 3]
|
||||||
|
};
|
||||||
|
const result = validateMcpMetadata(
|
||||||
|
JSON.stringify(complexMeta),
|
||||||
|
mockCreateErrorResponse
|
||||||
|
);
|
||||||
|
expect(result.parsedMetadata).toEqual(complexMeta);
|
||||||
|
expect(result.error).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should return parsed metadata for empty object', () => {
|
||||||
|
const result = validateMcpMetadata('{}', mockCreateErrorResponse);
|
||||||
|
expect(result.parsedMetadata).toEqual({});
|
||||||
|
expect(result.error).toBeUndefined();
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should return error for invalid JSON string', () => {
|
||||||
|
const result = validateMcpMetadata(
|
||||||
|
'{key: "value"}',
|
||||||
|
mockCreateErrorResponse
|
||||||
|
);
|
||||||
|
expect(result.error).toBeDefined();
|
||||||
|
expect(result.error?.isError).toBe(true);
|
||||||
|
expect(getErrorText(result.error)).toContain('Invalid metadata JSON');
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should return error for JSON array', () => {
|
||||||
|
const result = validateMcpMetadata('[1, 2, 3]', mockCreateErrorResponse);
|
||||||
|
expect(result.error).toBeDefined();
|
||||||
|
expect(result.error?.isError).toBe(true);
|
||||||
|
expect(getErrorText(result.error)).toContain(
|
||||||
|
'must be a JSON object (not null or array)'
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should return error for JSON null', () => {
|
||||||
|
const result = validateMcpMetadata('null', mockCreateErrorResponse);
|
||||||
|
expect(result.error).toBeDefined();
|
||||||
|
expect(result.error?.isError).toBe(true);
|
||||||
|
expect(getErrorText(result.error)).toContain(
|
||||||
|
'must be a JSON object (not null or array)'
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should return error for JSON string primitive', () => {
|
||||||
|
const result = validateMcpMetadata('"string"', mockCreateErrorResponse);
|
||||||
|
expect(result.error).toBeDefined();
|
||||||
|
expect(result.error?.isError).toBe(true);
|
||||||
|
expect(getErrorText(result.error)).toContain(
|
||||||
|
'must be a JSON object (not null or array)'
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should return error for JSON number primitive', () => {
|
||||||
|
const result = validateMcpMetadata('123', mockCreateErrorResponse);
|
||||||
|
expect(result.error).toBeDefined();
|
||||||
|
expect(result.error?.isError).toBe(true);
|
||||||
|
expect(getErrorText(result.error)).toContain(
|
||||||
|
'must be a JSON object (not null or array)'
|
||||||
|
);
|
||||||
|
});
|
||||||
|
|
||||||
|
it('should return error for JSON boolean primitive', () => {
|
||||||
|
const result = validateMcpMetadata('true', mockCreateErrorResponse);
|
||||||
|
expect(result.error).toBeDefined();
|
||||||
|
expect(result.error?.isError).toBe(true);
|
||||||
|
expect(getErrorText(result.error)).toContain(
|
||||||
|
'must be a JSON object (not null or array)'
|
||||||
|
);
|
||||||
|
});
|
||||||
|
});
|
||||||
|
});
|
||||||
|
|||||||
Reference in New Issue
Block a user