Address the schema architecture crisis

olaservo · olaservo · commit b555e41c0b3c · 2025-06-01T09:03:02.000-07:00
diff --git a/client/src/components/DynamicJsonForm.tsx b/client/src/components/DynamicJsonForm.tsx
@@ -113,6 +113,8 @@ const DynamicJsonForm = ({
     currentValue: JsonValue,
     path: string[] = [],
     depth: number = 0,
+    parentSchema?: JsonSchemaType,
+    propertyName?: string,
   ) => {
     if (
       depth >= maxDepth &&
@@ -122,7 +124,7 @@ const DynamicJsonForm = ({
       return (
         <JsonEditor
           value={JSON.stringify(
-            currentValue ?? generateDefaultValue(propSchema),
+            currentValue ?? generateDefaultValue(propSchema, propertyName, parentSchema),
             null,
             2,
           )}
@@ -140,6 +142,9 @@ const DynamicJsonForm = ({
       );
     }
 
+    // Check if this property is required in the parent schema
+    const isRequired = parentSchema?.required?.includes(propertyName || "") ?? false;
+
     switch (propSchema.type) {
       case "string":
         return (
@@ -148,16 +153,11 @@ const DynamicJsonForm = ({
             value={(currentValue as string) ?? ""}
             onChange={(e) => {
               const val = e.target.value;
-              // Allow clearing non-required fields by setting undefined
-              // This preserves the distinction between empty string and unset
-              if (!val && !propSchema.required) {
-                handleFieldChange(path, undefined);
-              } else {
-                handleFieldChange(path, val);
-              }
+              // Always allow setting string values, including empty strings
+              handleFieldChange(path, val);
             }}
             placeholder={propSchema.description}
-            required={propSchema.required}
+            required={isRequired}
           />
         );
       case "number":
@@ -167,9 +167,7 @@ const DynamicJsonForm = ({
             value={(currentValue as number)?.toString() ?? ""}
             onChange={(e) => {
               const val = e.target.value;
-              // Allow clearing non-required number fields
-              // This preserves the distinction between 0 and unset
-              if (!val && !propSchema.required) {
+              if (!val && !isRequired) {
                 handleFieldChange(path, undefined);
               } else {
                 const num = Number(val);
@@ -179,7 +177,7 @@ const DynamicJsonForm = ({
               }
             }}
             placeholder={propSchema.description}
-            required={propSchema.required}
+            required={isRequired}
           />
         );
       case "integer":
@@ -190,9 +188,7 @@ const DynamicJsonForm = ({
             value={(currentValue as number)?.toString() ?? ""}
             onChange={(e) => {
               const val = e.target.value;
-              // Allow clearing non-required integer fields
-              // This preserves the distinction between 0 and unset
-              if (!val && !propSchema.required) {
+              if (!val && !isRequired) {
                 handleFieldChange(path, undefined);
               } else {
                 const num = Number(val);
@@ -203,7 +199,7 @@ const DynamicJsonForm = ({
               }
             }}
             placeholder={propSchema.description}
-            required={propSchema.required}
+            required={isRequired}
           />
         );
       case "boolean":
@@ -213,7 +209,64 @@ const DynamicJsonForm = ({
             checked={(currentValue as boolean) ?? false}
             onChange={(e) => handleFieldChange(path, e.target.checked)}
             className="w-4 h-4"
-            required={propSchema.required}
+            required={isRequired}
+          />
+        );
+      case "object":
+        if (!propSchema.properties) {
+          return (
+            <JsonEditor
+              value={JSON.stringify(currentValue ?? {}, null, 2)}
+              onChange={(newValue) => {
+                try {
+                  const parsed = JSON.parse(newValue);
+                  handleFieldChange(path, parsed);
+                  setJsonError(undefined);
+                } catch (err) {
+                  setJsonError(err instanceof Error ? err.message : "Invalid JSON");
+                }
+              }}
+              error={jsonError}
+            />
+          );
+        }
+        
+        return (
+          <div className="space-y-2 border rounded p-3">
+            {Object.entries(propSchema.properties).map(([key, subSchema]) => (
+              <div key={key}>
+                <label className="block text-sm font-medium mb-1">
+                  {key}
+                  {propSchema.required?.includes(key) && (
+                    <span className="text-red-500 ml-1">*</span>
+                  )}
+                </label>
+                {renderFormFields(
+                  subSchema as JsonSchemaType,
+                  (currentValue as Record<string, JsonValue>)?.[key],
+                  [...path, key],
+                  depth + 1,
+                  propSchema,
+                  key,
+                )}
+              </div>
+            ))}
+          </div>
+        );
+      case "array":
+        return (
+          <JsonEditor
+            value={JSON.stringify(currentValue ?? [], null, 2)}
+            onChange={(newValue) => {
+              try {
+                const parsed = JSON.parse(newValue);
+                handleFieldChange(path, parsed);
+                setJsonError(undefined);
+              } catch (err) {
+                setJsonError(err instanceof Error ? err.message : "Invalid JSON");
+              }
+            }}
+            error={jsonError}
           />
         );
       default:
diff --git a/client/src/components/ToolsTab.tsx b/client/src/components/ToolsTab.tsx
@@ -7,7 +7,7 @@ import { TabsContent } from "@/components/ui/tabs";
 import { Textarea } from "@/components/ui/textarea";
 import DynamicJsonForm from "./DynamicJsonForm";
 import type { JsonValue, JsonSchemaType } from "@/utils/jsonUtils";
-import { generateDefaultValue } from "@/utils/schemaUtils";
+import { generateDefaultValue, isPropertyRequired } from "@/utils/schemaUtils";
 import {
   CompatibilityCallToolResult,
   ListToolsResult,
@@ -48,7 +48,7 @@ const ToolsTab = ({
       selectedTool?.inputSchema.properties ?? [],
     ).map(([key, value]) => [
       key,
-      generateDefaultValue(value as JsonSchemaType),
+      generateDefaultValue(value as JsonSchemaType, key, selectedTool?.inputSchema as JsonSchemaType),
     ]);
     setParams(Object.fromEntries(params));
   }, [selectedTool]);
@@ -92,13 +92,15 @@ const ToolsTab = ({
                 {Object.entries(selectedTool.inputSchema.properties ?? []).map(
                   ([key, value]) => {
                     const prop = value as JsonSchemaType;
+                    const inputSchema = selectedTool.inputSchema as JsonSchemaType;
+                    const required = isPropertyRequired(key, inputSchema);
                     return (
                       <div key={key}>
                         <Label
                           htmlFor={key}
                           className="block text-sm font-medium text-gray-700 dark:text-gray-300"
                         >
-                          {key}
+                          {key}{required && <span className="text-red-500 ml-1">*</span>}
                         </Label>
                         {prop.type === "boolean" ? (
                           <div className="flex items-center space-x-2 mt-2">
diff --git a/client/src/utils/jsonUtils.ts b/client/src/utils/jsonUtils.ts
@@ -17,7 +17,7 @@ export type JsonSchemaType = {
     | "object"
     | "null";
   description?: string;
-  required?: boolean;
+  required?: string[];
   default?: JsonValue;
   properties?: Record<string, JsonSchemaType>;
   items?: JsonSchemaType;
diff --git a/client/src/utils/schemaUtils.ts b/client/src/utils/schemaUtils.ts
@@ -81,46 +81,65 @@ export function hasOutputSchema(toolName: string): boolean {
 /**
  * Generates a default value based on a JSON schema type
  * @param schema The JSON schema definition
- * @returns A default value matching the schema type, or null for non-required fields
+ * @param propertyName Optional property name for checking if it's required in parent schema
+ * @param parentSchema Optional parent schema to check required array
+ * @returns A default value matching the schema type
  */
-export function generateDefaultValue(schema: JsonSchemaType): JsonValue {
-  if ("default" in schema) {
+export function generateDefaultValue(
+  schema: JsonSchemaType,
+  propertyName?: string,
+  parentSchema?: JsonSchemaType
+): JsonValue {
+  if ("default" in schema && schema.default !== undefined) {
     return schema.default;
   }
 
-  if (!schema.required) {
-    if (schema.type === "array") return [];
-    if (schema.type === "object") return {};
-    return undefined;
-  }
+  // Check if this property is required in the parent schema
+  const isRequired = parentSchema?.required?.includes(propertyName || "") ?? false;
 
   switch (schema.type) {
     case "string":
-      return "";
+      return isRequired ? "" : undefined;
     case "number":
     case "integer":
-      return 0;
+      return isRequired ? 0 : undefined;
     case "boolean":
-      return false;
+      return isRequired ? false : undefined;
     case "array":
       return [];
     case "object": {
       if (!schema.properties) return {};
 
       const obj: JsonObject = {};
-      Object.entries(schema.properties)
-        .filter(([, prop]) => prop.required)
-        .forEach(([key, prop]) => {
-          const value = generateDefaultValue(prop);
-          obj[key] = value;
-        });
+      // Only include properties that are required according to the schema's required array
+      Object.entries(schema.properties).forEach(([key, prop]) => {
+        const isPropertyRequired = schema.required?.includes(key) ?? false;
+        if (isPropertyRequired) {
+          const value = generateDefaultValue(prop, key, schema);
+          if (value !== undefined) {
+            obj[key] = value;
+          }
+        }
+      });
       return obj;
     }
-    default:
+    case "null":
       return null;
+    default:
+      return undefined;
   }
 }
 
+/**
+ * Helper function to check if a property is required in a schema
+ * @param propertyName The name of the property to check
+ * @param schema The parent schema containing the required array
+ * @returns true if the property is required, false otherwise
+ */
+export function isPropertyRequired(propertyName: string, schema: JsonSchemaType): boolean {
+  return schema.required?.includes(propertyName) ?? false;
+}
+
 /**
  * Formats a field key into a human-readable label
  * @param key The field key to format
