diff --git a/apps/api/src/api/tool_types.py b/apps/api/src/api/tool_types.py index acaa317..d669c39 100644 --- a/apps/api/src/api/tool_types.py +++ b/apps/api/src/api/tool_types.py @@ -50,7 +50,9 @@ class ToolTypeCreate(BaseModel): @classmethod def validate_definition_type(cls, v: str) -> str: if v not in ("compose", "dockerfile", "manifest"): - raise ValueError("definition_type must be 'compose', 'dockerfile', or 'manifest'") + raise ValueError( + "definition_type must be 'compose', 'dockerfile', or 'manifest'" + ) return v @field_validator("compose_template") @@ -59,10 +61,12 @@ class ToolTypeCreate(BaseModel): data = info.data if data.get("definition_type") != "compose": return v - + if v is None or not v.strip(): - raise ValueError("compose_template is required when definition_type is 'compose'") - + raise ValueError( + "compose_template is required when definition_type is 'compose'" + ) + validate_compose_yaml(v) return v @@ -72,13 +76,15 @@ class ToolTypeCreate(BaseModel): data = info.data if data.get("definition_type") != "dockerfile": return v - + if v is None or not v.strip(): - raise ValueError("dockerfile_template is required when definition_type is 'dockerfile'") - + raise ValueError( + "dockerfile_template is required when definition_type is 'dockerfile'" + ) + if not v.strip().startswith("FROM"): raise ValueError("Dockerfile must start with a FROM instruction") - + return v @field_validator("interface_type") @@ -104,44 +110,62 @@ class ToolTypeCreate(BaseModel): def validate_required_variables(cls, v: list[str], info) -> list[str]: if not v: return v - + data = info.data if data.get("definition_type") != "compose": return v - + template = data.get("compose_template") if not template: return v - + for var in v: placeholder = f"{{{{{var}}}}}" if placeholder not in template: - raise ValueError(f"Required variable '{var}' not found in compose template") - + raise ValueError( + f"Required variable '{var}' not found in compose template" + ) + return v @model_validator(mode="after") def validate_templates(self) -> "ToolTypeCreate": if self.definition_type == "manifest": if self.manifest_id is None: - raise ValueError("manifest_id is required when definition_type is 'manifest'") + raise ValueError( + "manifest_id is required when definition_type is 'manifest'" + ) return self - - if self.definition_type == "dockerfile" and (self.dockerfile_template is None or not self.dockerfile_template.strip()): - raise ValueError("dockerfile_template is required when definition_type is 'dockerfile'") - if self.definition_type == "compose" and (self.compose_template is None or not self.compose_template.strip()): - raise ValueError("compose_template is required when definition_type is 'compose'") - + + if self.definition_type == "dockerfile" and ( + self.dockerfile_template is None or not self.dockerfile_template.strip() + ): + raise ValueError( + "dockerfile_template is required when definition_type is 'dockerfile'" + ) + if self.definition_type == "compose" and ( + self.compose_template is None or not self.compose_template.strip() + ): + raise ValueError( + "compose_template is required when definition_type is 'compose'" + ) + # Validate that default_port is exposed in compose template (only if requires_port) - if self.requires_port and self.definition_type == "compose" and self.compose_template: + if ( + self.requires_port + and self.definition_type == "compose" + and self.compose_template + ): try: parsed = validate_compose_yaml(self.compose_template) except ValueError: return self - + if not check_port_exposed(parsed, self.default_port): - raise ValueError(f"Port {self.default_port} is not exposed in the compose template. Add it to the 'ports' section.") - + raise ValueError( + f"Port {self.default_port} is not exposed in the compose template. Add it to the 'ports' section." + ) + return self @@ -167,7 +191,9 @@ class ToolTypeUpdate(BaseModel): if v is None: return v if v not in ("compose", "dockerfile", "manifest"): - raise ValueError("definition_type must be 'compose', 'dockerfile', or 'manifest'") + raise ValueError( + "definition_type must be 'compose', 'dockerfile', or 'manifest'" + ) return v @field_validator("interface_type") @@ -198,15 +224,15 @@ class ToolTypeUpdate(BaseModel): def validate_dockerfile_template(cls, v: str | None, info) -> str | None: if v is None: return v - + data = info.data definition_type = data.get("definition_type") if definition_type and definition_type != "dockerfile": return v - + if not v.strip().startswith("FROM"): raise ValueError("Dockerfile must start with a FROM instruction") - + return v @@ -258,12 +284,15 @@ async def create_tool_type( """ user = await _get_user(session, user_id) await _require_admin(user) - + # Check for duplicate name existing = await session.scalar(select(ToolType).where(ToolType.name == data.name)) if existing: - raise HTTPException(status_code=status.HTTP_409_CONFLICT, detail="tool type with this name already exists") - + raise HTTPException( + status_code=status.HTTP_409_CONFLICT, + detail="tool type with this name already exists", + ) + tool_type = ToolType( name=data.name, display_name=data.display_name, @@ -336,7 +365,9 @@ async def get_tool_type( await _get_user(session, user_id) tool_type = await session.get(ToolType, tool_type_id) if tool_type is None: - raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="tool type not found") + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, detail="tool type not found" + ) return tool_type @@ -365,15 +396,17 @@ async def update_tool_type( """ user = await _get_user(session, user_id) await _require_admin(user) - + tool_type = await session.get(ToolType, tool_type_id) if tool_type is None: - raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="tool type not found") - + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, detail="tool type not found" + ) + # Built-in tool types can now be modified - + update_data = data.model_dump(exclude_unset=True) - + # Validate port if being updated requires_port = update_data.get("requires_port", tool_type.requires_port) if "default_port" in update_data and requires_port: @@ -381,9 +414,9 @@ async def update_tool_type( if new_port <= 0 or new_port > 65535: raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, - detail="Port must be between 1 and 65535" + detail="Port must be between 1 and 65535", ) - + # Only validate port exposure for compose definitions definition_type = update_data.get("definition_type", tool_type.definition_type) if definition_type == "compose": @@ -394,12 +427,11 @@ async def update_tool_type( if not check_port_exposed(parsed, new_port): raise HTTPException( status_code=status.HTTP_400_BAD_REQUEST, - detail=f"Port {new_port} is not exposed in the compose template" + detail=f"Port {new_port} is not exposed in the compose template", ) except ValueError as e: raise HTTPException( - status_code=status.HTTP_400_BAD_REQUEST, - detail=str(e) + status_code=status.HTTP_400_BAD_REQUEST, detail=str(e) ) # Validate required variables for compose definitions @@ -413,17 +445,17 @@ async def update_tool_type( template = tool_type.compose_template if template: validate_required_variables(template, update_data["required_variables"]) - + # When switching to manifest, clear legacy templates if definition_type == "manifest": if "manifest_id" in update_data: tool_type.manifest_id = update_data["manifest_id"] tool_type.compose_template = None tool_type.dockerfile_template = None - + for field, value in update_data.items(): setattr(tool_type, field, value) - + await session.commit() await session.refresh(tool_type) return tool_type @@ -509,10 +541,12 @@ async def validate_tool_type( await _get_user(session, user_id) tool_type = await session.get(ToolType, tool_type_id) if tool_type is None: - raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="tool type not found") - + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, detail="tool type not found" + ) + errors = [] - + if tool_type.definition_type == "compose": if not tool_type.compose_template: errors.append("Compose template is empty") @@ -521,17 +555,17 @@ async def validate_tool_type( validate_compose_yaml(tool_type.compose_template) except ValueError as e: errors.append(str(e)) - + elif tool_type.definition_type == "dockerfile": if not tool_type.dockerfile_template: errors.append("Dockerfile template is empty") elif not tool_type.dockerfile_template.strip().startswith("FROM"): errors.append("Dockerfile must start with a FROM instruction") - + elif tool_type.definition_type == "manifest": if not tool_type.manifest_id: errors.append("Manifest reference is missing") - + return { "valid": len(errors) == 0, "errors": errors, @@ -561,12 +595,14 @@ async def delete_tool_type( """ user = await _get_user(session, user_id) await _require_admin(user) - + tool_type = await session.get(ToolType, tool_type_id) if tool_type is None: - raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="tool type not found") - + raise HTTPException( + status_code=status.HTTP_404_NOT_FOUND, detail="tool type not found" + ) + # Built-in tool types can now be deleted - + await session.delete(tool_type) await session.commit() diff --git a/apps/web/src/components/manifest-editor.tsx b/apps/web/src/components/manifest-editor.tsx index 76a3b41..11a3111 100644 --- a/apps/web/src/components/manifest-editor.tsx +++ b/apps/web/src/components/manifest-editor.tsx @@ -1,4 +1,4 @@ -import { useState, useEffect, useCallback } from "react"; +import { useState, useEffect, useCallback, useRef } from "react"; import { Icon } from "./icon"; import { extractErrorMessage } from "../utils/errors"; import { @@ -170,11 +170,20 @@ export const ManifestEditor = ({ baseDefinitionId, ]); - // Notify parent of changes + // Notify parent of changes — only when built manifest actually differs + // from what we last sent, to avoid feedback loops with the manifest prop. + const lastSentRef = useRef(""); + const onChangeRef = useRef(onChange); + onChangeRef.current = onChange; + useEffect(() => { const m = buildManifest(); - onChange(m); - }, [buildManifest, onChange]); + const serialized = JSON.stringify(m); + if (serialized !== lastSentRef.current) { + lastSentRef.current = serialized; + onChangeRef.current(m); + } + }, [buildManifest]); const handlePreview = async () => { if (!definitionId) { diff --git a/apps/web/src/pages/tool-workshop.tsx b/apps/web/src/pages/tool-workshop.tsx index a098cfb..f80b218 100644 --- a/apps/web/src/pages/tool-workshop.tsx +++ b/apps/web/src/pages/tool-workshop.tsx @@ -1,8 +1,5 @@ import { useCallback, useEffect, useState } from "react"; -import { - ErrorState, - LoadingState, -} from "../components/data-states"; +import { ErrorState, LoadingState } from "../components/data-states"; import { Icon } from "../components/icon"; import { useMobileViewport } from "../hooks/use-mobile-viewport"; import { extractErrorMessage } from "../utils/errors"; @@ -75,7 +72,6 @@ export const ToolWorkshopPage = () => { const [toolTypeError, setToolTypeError] = useState(null); const [toolTypeDirty, setToolTypeDirty] = useState(false); - const selectedToolType = (toolTypes || []).find((t) => t.id === selectedToolTypeId) || null; @@ -215,7 +211,9 @@ export const ToolWorkshopPage = () => { return; } } else if (!manifestData) { - setToolTypeError("Manifest data is required for manifest definition type"); + setToolTypeError( + "Manifest data is required for manifest definition type", + ); return; } @@ -300,14 +298,12 @@ export const ToolWorkshopPage = () => { description: toolTypeForm.description.trim() || undefined, category: toolTypeForm.category.trim() || undefined, interface_type: toolTypeForm.interface_type, - base_image: - (manifestData.base_image as string) || undefined, + base_image: (manifestData.base_image as string) || undefined, base_definition_id: (manifestData.base_definition_id as string) || undefined, manifest: manifestData, }; - const newManifest = - await createToolDefinition(manifestPayload); + const newManifest = await createToolDefinition(manifestPayload); manifestId = newManifest.id; } } @@ -365,7 +361,6 @@ export const ToolWorkshopPage = () => { } }; - if (status === "loading") { return (
@@ -935,7 +930,7 @@ export const ToolWorkshopPage = () => { )}
- {( + {
{ )}
- )} - + } )}