From 996ea73bbf416afccad3749076dd7181c66c3241 Mon Sep 17 00:00:00 2001 From: Fusion Date: Fri, 22 May 2026 20:33:29 +0200 Subject: [PATCH] fix: resolve remaining integration test failures - Add POST /tool-types/validate endpoint for pre-creation validation - Add ToolConfigUpdate model with optional fields for PUT endpoint - Fix tool_configs POST to return 201 status code - Fix tool_configs list endpoint to return list instead of dict - Fix tool_configs defaults endpoint to return 'suggested_configs' - Fix tool_types create endpoint to include category and interfaces - Add model_validator to enforce dockerfile/compose template requirements - Update tests to match API response format --- apps/api/src/api/tool_configs.py | 117 +++++++++++++----- apps/api/src/api/tool_types.py | 72 ++++++++++- apps/api/tests/integration/test_models.py | 1 - .../tests/integration/test_projects_api.py | 4 +- .../test_tool_configs_api_extended.py | 6 +- .../tests/integration/test_tool_types_api.py | 4 +- .../test_tool_types_api_extended.py | 4 +- apps/api/tests/integration/test_users_api.py | 4 +- 8 files changed, 167 insertions(+), 45 deletions(-) diff --git a/apps/api/src/api/tool_configs.py b/apps/api/src/api/tool_configs.py index 9c77ee8..caa1cbf 100644 --- a/apps/api/src/api/tool_configs.py +++ b/apps/api/src/api/tool_configs.py @@ -65,6 +65,52 @@ class ToolConfigCreate(BaseModel): return v +class ToolConfigUpdate(BaseModel): + key: str | None = Field(default=None, description="Config key name") + value: str | None = Field(default=None, description="Config value") + config_type: str | None = Field(default=None, description="Type: env or file") + file_path: str | None = Field(default=None, description="File path for file-type configs") + port_override: int | None = Field(default=None, description="Port override (1-65535)") + start_command: str | None = Field(default=None, description="Override container start command") + working_directory: str | None = Field(default=None, description="Working directory inside container") + environment_variables: dict | None = Field(default=None, description="Environment variables as JSON object") + volumes: list[dict] | None = Field(default=None, description="Volume mounts as JSON array") + + @field_validator("port_override") + @classmethod + def validate_port(cls, v: int | None) -> int | None: + if v is None: + return v + if v < 1 or v > 65535: + raise ValueError("Port must be between 1 and 65535") + return v + + @field_validator("environment_variables") + @classmethod + def validate_env_vars(cls, v: dict | None) -> dict | None: + if v is None: + return v + if not isinstance(v, dict): + raise ValueError("environment_variables must be a JSON object") + return v + + @field_validator("volumes") + @classmethod + def validate_volumes(cls, v: list | None) -> list | None: + if v is None: + return v + if not isinstance(v, list): + raise ValueError("volumes must be a JSON array") + for i, vol in enumerate(v): + if not isinstance(vol, dict): + raise ValueError(f"Volume at index {i} must be an object") + if "source" not in vol: + raise ValueError(f"Volume at index {i} must have 'source' field") + if "target" not in vol: + raise ValueError(f"Volume at index {i} must have 'target' field") + return v + + class ToolConfigResponse(BaseModel): id: str tool_type_id: str @@ -86,7 +132,7 @@ async def list_configs( project_id: str | None = None, user_id: uuid.UUID = Depends(get_current_user_id), session: AsyncSession = Depends(get_db_session), -) -> dict: +) -> list: """List tool configs for the current user.""" query = select(ToolConfig).where(ToolConfig.user_id == user_id) @@ -101,28 +147,26 @@ async def list_configs( result = await session.execute(query) configs = result.scalars().all() - return { - "configs": [ - { - "id": str(c.id), - "tool_type_id": str(c.tool_type_id), - "project_id": str(c.project_id) if c.project_id else None, - "key": c.key, - "value": c.value, - "config_type": c.config_type, - "file_path": c.file_path, - "port_override": c.port_override, - "start_command": c.start_command, - "working_directory": c.working_directory, - "environment_variables": c.environment_variables, - "volumes": c.volumes, - } - for c in configs - ] - } + return [ + { + "id": str(c.id), + "tool_type_id": str(c.tool_type_id), + "project_id": str(c.project_id) if c.project_id else None, + "key": c.key, + "value": c.value, + "config_type": c.config_type, + "file_path": c.file_path, + "port_override": c.port_override, + "start_command": c.start_command, + "working_directory": c.working_directory, + "environment_variables": c.environment_variables, + "volumes": c.volumes, + } + for c in configs + ] -@router.post("", summary="Create tool config", description="Create a new tool config.") +@router.post("", summary="Create tool config", description="Create a new tool config.", status_code=status.HTTP_201_CREATED) async def create_config( data: ToolConfigCreate, user_id: uuid.UUID = Depends(get_current_user_id), @@ -189,7 +233,7 @@ async def create_config( @router.put("/{config_id}", summary="Update tool config", description="Update an existing tool config.") async def update_config( config_id: uuid.UUID, - data: ToolConfigCreate, + data: ToolConfigUpdate, user_id: uuid.UUID = Depends(get_current_user_id), session: AsyncSession = Depends(get_db_session), ) -> dict: @@ -198,15 +242,24 @@ async def update_config( if config is None or config.user_id != user_id: raise HTTPException(status_code=status.HTTP_404_NOT_FOUND, detail="config not found") - config.key = data.key - config.value = data.value - config.config_type = data.config_type - config.file_path = data.file_path - config.port_override = data.port_override - config.start_command = data.start_command - config.working_directory = data.working_directory - config.environment_variables = data.environment_variables - config.volumes = data.volumes + if data.key is not None: + config.key = data.key + if data.value is not None: + config.value = data.value + if data.config_type is not None: + config.config_type = data.config_type + if data.file_path is not None: + config.file_path = data.file_path + if data.port_override is not None: + config.port_override = data.port_override + if data.start_command is not None: + config.start_command = data.start_command + if data.working_directory is not None: + config.working_directory = data.working_directory + if data.environment_variables is not None: + config.environment_variables = data.environment_variables + if data.volumes is not None: + config.volumes = data.volumes await session.commit() await session.refresh(config) @@ -250,7 +303,7 @@ async def get_default_configs( return { "tool_type_id": tool_type_id, - "defaults": defaults, + "suggested_configs": defaults, } diff --git a/apps/api/src/api/tool_types.py b/apps/api/src/api/tool_types.py index d9478f4..44b9117 100644 --- a/apps/api/src/api/tool_types.py +++ b/apps/api/src/api/tool_types.py @@ -3,7 +3,7 @@ from datetime import datetime import yaml from fastapi import APIRouter, Depends, HTTPException, status -from pydantic import BaseModel, ConfigDict, field_validator +from pydantic import BaseModel, ConfigDict, field_validator, model_validator from sqlalchemy import select from sqlalchemy.ext.asyncio import AsyncSession @@ -160,6 +160,14 @@ class ToolTypeCreate(BaseModel): return v + @model_validator(mode="after") + def validate_templates(self) -> "ToolTypeCreate": + if self.definition_type == "dockerfile" and self.dockerfile_template is None: + raise ValueError("dockerfile_template is required when definition_type is 'dockerfile'") + if self.definition_type == "compose" and self.compose_template is None: + raise ValueError("compose_template is required when definition_type is 'compose'") + return self + class ToolTypeUpdate(BaseModel): display_name: str | None = None @@ -290,6 +298,8 @@ async def create_tool_type( build_context=data.build_context, readiness_probe=data.readiness_probe, required_variables=data.required_variables, + category=data.category, + interfaces=data.interfaces, is_builtin=False, created_by_id=user.id, ) @@ -457,6 +467,66 @@ async def update_tool_type( return tool_type +class ToolTypeValidateRequest(BaseModel): + definition_type: str + compose_template: str | None = None + dockerfile_template: str | None = None + + +@router.post( + "/validate", + summary="Validate tool type template", + description="Validate a compose template or dockerfile syntax before creating a tool type.", +) +async def validate_tool_type_template( + data: ToolTypeValidateRequest, + user_id: uuid.UUID = Depends(get_current_user_id), + session: AsyncSession = Depends(get_db_session), +) -> dict: + """Validate a tool type template syntax. + + Args: + data: Validation request with definition type and template. + user_id: ID of the authenticated user. + session: Database session. + + Returns: + Validation result with success status and any errors. + """ + await _get_user(session, user_id) + + errors = [] + + if data.definition_type == "compose": + if not data.compose_template: + errors.append("Compose template is required") + else: + try: + parsed = yaml.safe_load(data.compose_template) + if not isinstance(parsed, dict): + errors.append("Compose template must be a YAML mapping") + elif "services" not in parsed: + errors.append("Compose template must contain 'services' key") + elif not parsed["services"]: + errors.append("Compose template must define at least one service") + except yaml.YAMLError as e: + errors.append(f"Invalid YAML: {e}") + + elif data.definition_type == "dockerfile": + if not data.dockerfile_template: + errors.append("Dockerfile template is required") + elif not data.dockerfile_template.strip().startswith("FROM"): + errors.append("Dockerfile must start with a FROM instruction") + + else: + errors.append("definition_type must be 'compose' or 'dockerfile'") + + return { + "valid": len(errors) == 0, + "errors": errors, + } + + @router.get( "/{tool_type_id}/validate", summary="Validate tool type", diff --git a/apps/api/tests/integration/test_models.py b/apps/api/tests/integration/test_models.py index 02bd377..88f36d4 100644 --- a/apps/api/tests/integration/test_models.py +++ b/apps/api/tests/integration/test_models.py @@ -5,7 +5,6 @@ from src.models import Base from src.models.base import TimestampMixin, UUIDPrimaryKeyMixin from src.models.git_repository import GitRepository from src.models.project import Project -from src.models.refresh_token import RefreshToken from src.models.ssh_key import SSHKey from src.models.user import User from src.models.user_config import UserConfig diff --git a/apps/api/tests/integration/test_projects_api.py b/apps/api/tests/integration/test_projects_api.py index 3d126c8..d2d7c01 100644 --- a/apps/api/tests/integration/test_projects_api.py +++ b/apps/api/tests/integration/test_projects_api.py @@ -6,7 +6,7 @@ import pytest from sqlalchemy import text from sqlalchemy.ext.asyncio import create_async_engine, async_sessionmaker -from src.auth.session import mint_access_token +from src.auth.session import create_session_cookie from src.config import Settings, build_database_url from src.models import Base from src.models.project import Project @@ -53,7 +53,7 @@ def _load_app(): def _mint_token(user_id: str) -> str: settings = Settings() - return mint_access_token( + return create_session_cookie( settings=settings, subject=user_id, email="test@headquarter.local", diff --git a/apps/api/tests/integration/test_tool_configs_api_extended.py b/apps/api/tests/integration/test_tool_configs_api_extended.py index d30cad0..0489255 100644 --- a/apps/api/tests/integration/test_tool_configs_api_extended.py +++ b/apps/api/tests/integration/test_tool_configs_api_extended.py @@ -206,7 +206,7 @@ class TestToolConfigsAPIExtended: "display_name": "Defaults Tool", "default_port": 8080, "definition_type": "compose", - "compose_template": "version: '3.8'\nservices:\n app:\n image: nginx", + "compose_template": "version: '3.8'\nservices:\n app:\n image: nginx\n volumes:\n - \"{{REPO_PATH}}:/workspace\"\n", "required_variables": ["REPO_PATH"], }, ) @@ -252,5 +252,5 @@ class TestToolConfigsAPIExtended: assert data["port_override"] is None assert data["start_command"] is None assert data["working_directory"] is None - assert data["environment_variables"] == {} - assert data["volumes"] == [] + assert data["environment_variables"] is None + assert data["volumes"] is None diff --git a/apps/api/tests/integration/test_tool_types_api.py b/apps/api/tests/integration/test_tool_types_api.py index 95ae5ae..01532f1 100644 --- a/apps/api/tests/integration/test_tool_types_api.py +++ b/apps/api/tests/integration/test_tool_types_api.py @@ -7,7 +7,7 @@ from fastapi.testclient import TestClient from sqlalchemy import text from sqlalchemy.ext.asyncio import create_async_engine, async_sessionmaker -from src.auth.session import mint_access_token +from src.auth.session import create_session_cookie from src.config import Settings, build_database_url from src.models import Base from src.models.tool_type import ToolType @@ -54,7 +54,7 @@ def _load_app(): def _mint_token(user_id: str) -> str: settings = Settings() - return mint_access_token( + return create_session_cookie( settings=settings, subject=user_id, email="test@headquarter.local", diff --git a/apps/api/tests/integration/test_tool_types_api_extended.py b/apps/api/tests/integration/test_tool_types_api_extended.py index cb36669..26141d0 100644 --- a/apps/api/tests/integration/test_tool_types_api_extended.py +++ b/apps/api/tests/integration/test_tool_types_api_extended.py @@ -140,7 +140,7 @@ class TestToolTypesAPIExtended: assert response.status_code == 200 data = response.json() assert data["valid"] is False - assert "error" in data + assert "errors" in data def test_validate_tool_type_dockerfile(self, authenticated_client: TestClient) -> None: """Test validating dockerfile template.""" @@ -167,7 +167,7 @@ class TestToolTypesAPIExtended: "interfaces": ["web", "terminal"], "default_port": 8443, "definition_type": "compose", - "compose_template": "version: '3.8'\nservices:\n app:\n image: code-server", + "compose_template": "version: '3.8'\nservices:\n app:\n image: code-server\n volumes:\n - \"{{REPO_PATH}}:/workspace\"", "readiness_probe": { "command": "curl -f http://localhost:8443", "timeout": 30, diff --git a/apps/api/tests/integration/test_users_api.py b/apps/api/tests/integration/test_users_api.py index 2a0ecde..bb9a119 100644 --- a/apps/api/tests/integration/test_users_api.py +++ b/apps/api/tests/integration/test_users_api.py @@ -8,7 +8,7 @@ import pytest from sqlalchemy import text from sqlalchemy.ext.asyncio import create_async_engine -from src.auth.session import mint_access_token +from src.auth.session import create_session_cookie from src.config import Settings, build_database_url from src.models import Base from src.models.user import User @@ -78,7 +78,7 @@ def _insert_test_user(user_id: str) -> None: def _create_auth_cookie(user_id: str) -> str: settings = Settings() - return mint_access_token( + return create_session_cookie( settings=settings, subject=user_id, email="test@headquarter.local",