Add input validation guidelines and enhance cleanParams function to preserve default values
This commit is contained in:
parent
b025bffc9d
commit
0123a71da4
3 changed files with 94 additions and 1 deletions
11
CLAUDE.md
11
CLAUDE.md
|
|
@ -31,3 +31,14 @@ The project is organized as a monorepo with workspaces:
|
|||
- `client/`: React frontend with Vite, TypeScript and Tailwind
|
||||
- `server/`: Express backend with TypeScript
|
||||
- `cli/`: Command-line interface for testing and invoking MCP server methods directly
|
||||
|
||||
## Tool Input Validation Guidelines
|
||||
|
||||
When handling tool input parameters and form fields:
|
||||
|
||||
- **Optional fields with empty values should be omitted entirely** - Do not send empty strings or null values for optional parameters, UNLESS the field has an explicit default value in the schema that matches the current value
|
||||
- **Fields with explicit defaults should preserve their default values** - If a field has an explicit default in its schema (e.g., `default: null`), and the current value matches that default, include it in the request. This is a meaningful value the tool expects
|
||||
- **Required fields should preserve their values even when empty** - This allows the server to properly validate and return appropriate error messages
|
||||
- **Deeper validation should be handled by the server** - Inspector should focus on basic field presence, while the MCP server handles parameter validation according to its schema
|
||||
|
||||
These guidelines ensure clean parameter passing and proper separation of concerns between the Inspector client and MCP servers.
|
||||
|
|
|
|||
|
|
@ -205,4 +205,76 @@ describe("cleanParams", () => {
|
|||
// optionalField omitted entirely
|
||||
});
|
||||
});
|
||||
|
||||
it("should preserve null values when field has default: null", () => {
|
||||
const schema: JsonSchemaType = {
|
||||
type: "object",
|
||||
required: [],
|
||||
properties: {
|
||||
optionalFieldWithNullDefault: { type: "string", default: null },
|
||||
optionalFieldWithoutDefault: { type: "string" },
|
||||
},
|
||||
};
|
||||
|
||||
const params = {
|
||||
optionalFieldWithNullDefault: null,
|
||||
optionalFieldWithoutDefault: null,
|
||||
};
|
||||
|
||||
const cleaned = cleanParams(params, schema);
|
||||
|
||||
expect(cleaned).toEqual({
|
||||
optionalFieldWithNullDefault: null, // preserved because default: null
|
||||
// optionalFieldWithoutDefault omitted
|
||||
});
|
||||
});
|
||||
|
||||
it("should preserve default values that match current value", () => {
|
||||
const schema: JsonSchemaType = {
|
||||
type: "object",
|
||||
required: [],
|
||||
properties: {
|
||||
fieldWithDefaultString: { type: "string", default: "defaultValue" },
|
||||
fieldWithDefaultNumber: { type: "number", default: 42 },
|
||||
fieldWithDefaultNull: { type: "string", default: null },
|
||||
fieldWithDefaultBoolean: { type: "boolean", default: false },
|
||||
},
|
||||
};
|
||||
|
||||
const params = {
|
||||
fieldWithDefaultString: "defaultValue",
|
||||
fieldWithDefaultNumber: 42,
|
||||
fieldWithDefaultNull: null,
|
||||
fieldWithDefaultBoolean: false,
|
||||
};
|
||||
|
||||
const cleaned = cleanParams(params, schema);
|
||||
|
||||
expect(cleaned).toEqual({
|
||||
fieldWithDefaultString: "defaultValue",
|
||||
fieldWithDefaultNumber: 42,
|
||||
fieldWithDefaultNull: null,
|
||||
fieldWithDefaultBoolean: false,
|
||||
});
|
||||
});
|
||||
|
||||
it("should omit values that do not match their default", () => {
|
||||
const schema: JsonSchemaType = {
|
||||
type: "object",
|
||||
required: [],
|
||||
properties: {
|
||||
fieldWithDefault: { type: "string", default: "defaultValue" },
|
||||
},
|
||||
};
|
||||
|
||||
const params = {
|
||||
fieldWithDefault: null, // doesn't match default
|
||||
};
|
||||
|
||||
const cleaned = cleanParams(params, schema);
|
||||
|
||||
expect(cleaned).toEqual({
|
||||
// fieldWithDefault omitted because value (null) doesn't match default ("defaultValue")
|
||||
});
|
||||
});
|
||||
});
|
||||
|
|
|
|||
|
|
@ -2,7 +2,7 @@ import type { JsonSchemaType } from "./jsonUtils";
|
|||
|
||||
/**
|
||||
* Cleans parameters by removing undefined, null, and empty string values for optional fields
|
||||
* while preserving all values for required fields.
|
||||
* while preserving all values for required fields and fields with explicit default values.
|
||||
*
|
||||
* @param params - The parameters object to clean
|
||||
* @param schema - The JSON schema defining which fields are required
|
||||
|
|
@ -14,13 +14,23 @@ export function cleanParams(
|
|||
): Record<string, unknown> {
|
||||
const cleaned: Record<string, unknown> = {};
|
||||
const required = schema.required || [];
|
||||
const properties = schema.properties || {};
|
||||
|
||||
for (const [key, value] of Object.entries(params)) {
|
||||
const isFieldRequired = required.includes(key);
|
||||
const fieldSchema = properties[key] as JsonSchemaType | undefined;
|
||||
|
||||
// Check if the field has an explicit default value
|
||||
const hasDefault = fieldSchema && "default" in fieldSchema;
|
||||
const defaultValue = hasDefault ? fieldSchema.default : undefined;
|
||||
|
||||
if (isFieldRequired) {
|
||||
// Required fields: always include, even if empty string or falsy
|
||||
cleaned[key] = value;
|
||||
} else if (hasDefault && value === defaultValue) {
|
||||
// Field has a default value and current value matches it - preserve it
|
||||
// This is important for cases like default: null
|
||||
cleaned[key] = value;
|
||||
} else {
|
||||
// Optional fields: only include if they have meaningful values
|
||||
if (value !== undefined && value !== "" && value !== null) {
|
||||
|
|
|
|||
Loading…
Reference in a new issue