fix: support manifest-type tool definitions in Tool Workshop
Backend: - Allow 'manifest' in tool_types definition_type validators - Add manifest_id to ToolTypeCreate, ToolTypeUpdate, ToolTypeResponse - Skip compose/dockerfile template validation when definition_type is manifest - Require manifest_id when definition_type is manifest - Clear legacy templates when switching to manifest type Frontend: - Load manifest data via getToolDefinition when selecting a manifest-type tool - Create/update manifest definition via tool-definitions API when saving - Pass manifest_id to tool-types create/update API - Fix unused EmptyState import after configs/folders cleanup
This commit is contained in:
@@ -35,6 +35,7 @@ class ToolTypeCreate(BaseModel):
|
||||
description: str | None = None
|
||||
default_port: int = 0
|
||||
definition_type: str = "compose"
|
||||
manifest_id: uuid.UUID | None = None
|
||||
compose_template: str | None = None
|
||||
dockerfile_template: str | None = None
|
||||
build_context: dict | None = None
|
||||
@@ -48,8 +49,8 @@ class ToolTypeCreate(BaseModel):
|
||||
@field_validator("definition_type")
|
||||
@classmethod
|
||||
def validate_definition_type(cls, v: str) -> str:
|
||||
if v not in ("compose", "dockerfile"):
|
||||
raise ValueError("definition_type must be 'compose' or 'dockerfile'")
|
||||
if v not in ("compose", "dockerfile", "manifest"):
|
||||
raise ValueError("definition_type must be 'compose', 'dockerfile', or 'manifest'")
|
||||
return v
|
||||
|
||||
@field_validator("compose_template")
|
||||
@@ -59,7 +60,7 @@ class ToolTypeCreate(BaseModel):
|
||||
if data.get("definition_type") != "compose":
|
||||
return v
|
||||
|
||||
if v is None:
|
||||
if v is None or not v.strip():
|
||||
raise ValueError("compose_template is required when definition_type is 'compose'")
|
||||
|
||||
validate_compose_yaml(v)
|
||||
@@ -72,7 +73,7 @@ class ToolTypeCreate(BaseModel):
|
||||
if data.get("definition_type") != "dockerfile":
|
||||
return v
|
||||
|
||||
if v is None:
|
||||
if v is None or not v.strip():
|
||||
raise ValueError("dockerfile_template is required when definition_type is 'dockerfile'")
|
||||
|
||||
if not v.strip().startswith("FROM"):
|
||||
@@ -121,9 +122,14 @@ class ToolTypeCreate(BaseModel):
|
||||
|
||||
@model_validator(mode="after")
|
||||
def validate_templates(self) -> "ToolTypeCreate":
|
||||
if self.definition_type == "dockerfile" and self.dockerfile_template is None:
|
||||
if self.definition_type == "manifest":
|
||||
if self.manifest_id is None:
|
||||
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:
|
||||
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)
|
||||
@@ -144,6 +150,7 @@ class ToolTypeUpdate(BaseModel):
|
||||
description: str | None = None
|
||||
default_port: int | None = None
|
||||
definition_type: str | None = None
|
||||
manifest_id: uuid.UUID | None = None
|
||||
compose_template: str | None = None
|
||||
dockerfile_template: str | None = None
|
||||
build_context: dict | None = None
|
||||
@@ -159,8 +166,8 @@ class ToolTypeUpdate(BaseModel):
|
||||
def validate_definition_type(cls, v: str | None) -> str | None:
|
||||
if v is None:
|
||||
return v
|
||||
if v not in ("compose", "dockerfile"):
|
||||
raise ValueError("definition_type must be 'compose' or 'dockerfile'")
|
||||
if v not in ("compose", "dockerfile", "manifest"):
|
||||
raise ValueError("definition_type must be 'compose', 'dockerfile', or 'manifest'")
|
||||
return v
|
||||
|
||||
@field_validator("interface_type")
|
||||
@@ -215,6 +222,7 @@ class ToolTypeResponse(BaseModel):
|
||||
requires_port: bool
|
||||
default_port: int
|
||||
definition_type: str
|
||||
manifest_id: uuid.UUID | None
|
||||
compose_template: str | None
|
||||
dockerfile_template: str | None
|
||||
build_context: dict | None
|
||||
@@ -262,6 +270,7 @@ async def create_tool_type(
|
||||
description=data.description,
|
||||
default_port=data.default_port,
|
||||
definition_type=data.definition_type,
|
||||
manifest_id=data.manifest_id,
|
||||
compose_template=data.compose_template,
|
||||
dockerfile_template=data.dockerfile_template,
|
||||
build_context=data.build_context,
|
||||
@@ -405,6 +414,13 @@ async def update_tool_type(
|
||||
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)
|
||||
|
||||
@@ -458,8 +474,11 @@ async def validate_tool_type_template(
|
||||
elif not data.dockerfile_template.strip().startswith("FROM"):
|
||||
errors.append("Dockerfile must start with a FROM instruction")
|
||||
|
||||
elif data.definition_type == "manifest":
|
||||
pass # Manifest validation is handled separately
|
||||
|
||||
else:
|
||||
errors.append("definition_type must be 'compose' or 'dockerfile'")
|
||||
errors.append("definition_type must be 'compose', 'dockerfile', or 'manifest'")
|
||||
|
||||
return {
|
||||
"valid": len(errors) == 0,
|
||||
@@ -509,6 +528,10 @@ async def validate_tool_type(
|
||||
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,
|
||||
|
||||
@@ -1,6 +1,5 @@
|
||||
import { useCallback, useEffect, useState } from "react";
|
||||
import {
|
||||
EmptyState,
|
||||
ErrorState,
|
||||
LoadingState,
|
||||
} from "../components/data-states";
|
||||
@@ -22,7 +21,10 @@ import {
|
||||
type UpdateToolTypeRequest,
|
||||
} from "../api/tool_types";
|
||||
import {
|
||||
createToolDefinition,
|
||||
getToolDefinition,
|
||||
listToolDefinitions,
|
||||
updateToolDefinition,
|
||||
type ToolDefinitionManifest,
|
||||
} from "../api/tool_definitions";
|
||||
import { ManifestEditor } from "../components/manifest-editor";
|
||||
@@ -120,7 +122,7 @@ export const ToolWorkshopPage = () => {
|
||||
setManifestDefinitionId(null);
|
||||
};
|
||||
|
||||
const populateToolTypeForm = (toolType: ToolType) => {
|
||||
const populateToolTypeForm = async (toolType: ToolType) => {
|
||||
setToolTypeForm({
|
||||
name: toolType.name,
|
||||
display_name: toolType.display_name,
|
||||
@@ -142,9 +144,18 @@ export const ToolWorkshopPage = () => {
|
||||
});
|
||||
setToolTypeError(null);
|
||||
setToolTypeDirty(false);
|
||||
// For manifest types, we'd need to load the manifest data separately
|
||||
setManifestData(null);
|
||||
setManifestDefinitionId(toolType.manifest_id || null);
|
||||
|
||||
if (toolType.definition_type === "manifest" && toolType.manifest_id) {
|
||||
try {
|
||||
const defn = await getToolDefinition(toolType.manifest_id);
|
||||
setManifestData(defn.manifest);
|
||||
} catch {
|
||||
setManifestData(null);
|
||||
}
|
||||
} else {
|
||||
setManifestData(null);
|
||||
}
|
||||
};
|
||||
|
||||
const handleSelectToolType = (toolType: ToolType | null) => {
|
||||
@@ -203,6 +214,9 @@ export const ToolWorkshopPage = () => {
|
||||
);
|
||||
return;
|
||||
}
|
||||
} else if (!manifestData) {
|
||||
setToolTypeError("Manifest data is required for manifest definition type");
|
||||
return;
|
||||
}
|
||||
|
||||
const variables = toolTypeForm.required_variables
|
||||
@@ -226,6 +240,23 @@ export const ToolWorkshopPage = () => {
|
||||
|
||||
try {
|
||||
if (isCreating) {
|
||||
let manifestId: string | undefined;
|
||||
if (toolTypeForm.definition_type === "manifest" && manifestData) {
|
||||
const manifestPayload = {
|
||||
name: toolTypeForm.name.trim(),
|
||||
display_name: toolTypeForm.display_name.trim(),
|
||||
description: toolTypeForm.description.trim() || undefined,
|
||||
category: toolTypeForm.category.trim() || undefined,
|
||||
interface_type: toolTypeForm.interface_type,
|
||||
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);
|
||||
manifestId = newManifest.id;
|
||||
}
|
||||
|
||||
const input: CreateToolTypeRequest = {
|
||||
name: toolTypeForm.name.trim(),
|
||||
display_name: toolTypeForm.display_name.trim(),
|
||||
@@ -237,6 +268,7 @@ export const ToolWorkshopPage = () => {
|
||||
? Number(toolTypeForm.default_port)
|
||||
: 0,
|
||||
definition_type: toolTypeForm.definition_type,
|
||||
manifest_id: manifestId,
|
||||
compose_template:
|
||||
toolTypeForm.definition_type === "compose" ? template : undefined,
|
||||
dockerfile_template:
|
||||
@@ -252,6 +284,34 @@ export const ToolWorkshopPage = () => {
|
||||
setSelectedToolTypeId(newTool.id);
|
||||
setToolTypeDirty(false);
|
||||
} else if (selectedToolType) {
|
||||
let manifestId = selectedToolType.manifest_id || undefined;
|
||||
if (toolTypeForm.definition_type === "manifest" && manifestData) {
|
||||
if (manifestId) {
|
||||
await updateToolDefinition(manifestId, {
|
||||
display_name: toolTypeForm.display_name.trim(),
|
||||
description: toolTypeForm.description.trim() || undefined,
|
||||
category: toolTypeForm.category.trim() || undefined,
|
||||
manifest: manifestData,
|
||||
});
|
||||
} else {
|
||||
const manifestPayload = {
|
||||
name: toolTypeForm.name.trim(),
|
||||
display_name: toolTypeForm.display_name.trim(),
|
||||
description: toolTypeForm.description.trim() || undefined,
|
||||
category: toolTypeForm.category.trim() || undefined,
|
||||
interface_type: toolTypeForm.interface_type,
|
||||
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);
|
||||
manifestId = newManifest.id;
|
||||
}
|
||||
}
|
||||
|
||||
const input: UpdateToolTypeRequest = {
|
||||
display_name: toolTypeForm.display_name.trim(),
|
||||
description: toolTypeForm.description.trim() || undefined,
|
||||
@@ -262,6 +322,10 @@ export const ToolWorkshopPage = () => {
|
||||
? Number(toolTypeForm.default_port)
|
||||
: 0,
|
||||
definition_type: toolTypeForm.definition_type,
|
||||
manifest_id:
|
||||
toolTypeForm.definition_type === "manifest"
|
||||
? manifestId
|
||||
: undefined,
|
||||
compose_template:
|
||||
toolTypeForm.definition_type === "compose" ? template : undefined,
|
||||
dockerfile_template:
|
||||
|
||||
Reference in New Issue
Block a user