FIX: Register created converters and targets only after the response is built

pyrit/backend/services/converter_service.py

@@ -173,8 +173,9 @@ async def create_converter_async(self, *, request: CreateConverterRequest) -> Co
         """
         Create a new converter instance from API request.
 
-        Instantiates the converter with the given type and params,
-        then registers it in the registry.
+        Instantiates the converter with the given type and params and builds the
+        response before registering it, so a request that fails at any step leaves
+        no registered converter and removes its uploaded files.
 
         Args:
             request: The create converter request with type and params.
@@ -194,20 +195,20 @@ async def create_converter_async(self, *, request: CreateConverterRequest) -> Co
             params=request.params,
         )
         try:
-            converter_obj = self._registry.create_named_instance(
+            # Uploads may have yielded to another request that took the name.
+            self._registry.instances.validate_name_available(request.name)
+            converter_obj = self._registry.create_instance(request.type, **params)
+            converter = self._build_instance_from_object(converter_id=request.name, converter_obj=converter_obj)
+            self._registry.instances.register(
+                converter_obj,
                 name=request.name,
-                type_name=request.type,
-                params=params,
-                registry_metadata={_OWNED_ARTIFACT_PATHS_KEY: [str(path) for path in owned_paths]},
+                metadata={_OWNED_ARTIFACT_PATHS_KEY: [str(path) for path in owned_paths]},
             )
         except (Exception, asyncio.CancelledError):
             await self._remove_owned_artifacts_async(paths=owned_paths)
             raise
 
-        return self._build_instance_from_object(
-            converter_id=request.name,
-            converter_obj=converter_obj,
-        )
+        return converter
 
     async def preview_conversion_async(self, *, request: ConverterPreviewRequest) -> ConverterPreviewResponse:
         """

pyrit/backend/services/target_service.py

@@ -183,7 +183,8 @@ async def create_target_async(self, *, request: CreateTargetRequest) -> TargetIn
         by the target classes themselves. This service only enforces the
         request-level auth contract: for ``identity`` it confirms the target
         supports it and omits the api_key so the target validates its own
-        endpoint and authenticates itself.
+        endpoint and authenticates itself. The response is built before the
+        target is registered, so a failed request leaves no registered target.
 
         Args:
             request: The create target request with type, params, and auth_mode.
@@ -214,12 +215,11 @@ async def create_target_async(self, *, request: CreateTargetRequest) -> TargetIn
         # LEGACY COMPATIBILITY: The current configuration UI omits the name.
         # Remove this generated fallback after that UI sends an explicit name.
         target_registry_name = request.name or f"compat_{uuid.uuid4().hex}"
-        target_obj = self._registry.create_named_instance(
-            name=target_registry_name,
-            type_name=request.type,
-            params=params,
-        )
-        return self._build_instance_from_object(target_registry_name=target_registry_name, target_obj=target_obj)
+        self._registry.instances.validate_name_available(target_registry_name)
+        target_obj = self._registry.create_instance(request.type, **params)
+        target = self._build_instance_from_object(target_registry_name=target_registry_name, target_obj=target_obj)
+        self._registry.instances.register(target_obj, name=target_registry_name)
+        return target
 
 
 @lru_cache(maxsize=1)

tests/unit/backend/test_converter_service.py

@@ -783,6 +783,69 @@ async def test_create_converter_cleans_upload_when_construction_fails(
         assert service._registry.instances.get("invalid-pdf") is None
         assert list(service._upload_path.iterdir()) == []
 
+    async def test_create_converter_registers_nothing_when_response_mapping_fails(
+        self, upload_service: ConverterService
+    ) -> None:
+        request = CreateConverterRequest(
+            name="unmapped-pdf",
+            type="PDFConverter",
+            params={"existing_pdf": _make_data_uri(mime_type="application/pdf", content=b"%PDF-1.4\n")},
+        )
+
+        with (
+            patch(
+                "pyrit.backend.services.converter_service.converter_object_to_instance",
+                side_effect=RuntimeError("mapping failed"),
+            ),
+            pytest.raises(RuntimeError, match="mapping failed"),
+        ):
+            await upload_service.create_converter_async(request=request)
+
+        assert upload_service._registry.instances.get("unmapped-pdf") is None
+        assert list(upload_service._upload_path.iterdir()) == []
+
+    async def test_create_converter_rechecks_name_after_upload(self, upload_service: ConverterService) -> None:
+        persist_async = upload_service._persist_data_uri_params_async
+
+        async def persist_then_take_name_async(**kwargs: object) -> object:
+            result = await persist_async(**kwargs)
+            upload_service._registry.instances.register(Base64Converter(), name="taken")
+            return result
+
+        request = CreateConverterRequest(
+            name="taken",
+            type="PDFConverter",
+            params={"existing_pdf": _make_data_uri(mime_type="application/pdf", content=b"%PDF-1.4\n")},
+        )
+        with (
+            patch.object(upload_service, "_persist_data_uri_params_async", side_effect=persist_then_take_name_async),
+            patch.object(upload_service._registry, "create_instance") as create_instance,
+            pytest.raises(ValueError, match="already exists"),
+        ):
+            await upload_service.create_converter_async(request=request)
+
+        create_instance.assert_not_called()
+        assert list(upload_service._upload_path.iterdir()) == []
+
+    async def test_create_converter_removes_upload_when_registration_fails(
+        self, upload_service: ConverterService
+    ) -> None:
+        request = CreateConverterRequest(
+            name="raced-pdf",
+            type="PDFConverter",
+            params={"existing_pdf": _make_data_uri(mime_type="application/pdf", content=b"%PDF-1.4\n")},
+        )
+
+        with (
+            patch.object(
+                upload_service._registry.instances, "register", side_effect=ValueError("'raced-pdf' already exists")
+            ),
+            pytest.raises(ValueError, match="already exists"),
+        ):
+            await upload_service.create_converter_async(request=request)
+
+        assert list(upload_service._upload_path.iterdir()) == []
+
     @pytest.mark.parametrize("error", [OSError("write failed"), asyncio.CancelledError()])
     async def test_persist_data_uri_cleans_partial_write(
         self, upload_service: ConverterService, error: BaseException

tests/unit/backend/test_target_service.py

@@ -415,6 +415,22 @@ async def test_create_target_rejects_reserved_route_name(self, sqlite_instance,
                 request=CreateTargetRequest(name=name, type="TextTarget", params={}),
             )
 
+    async def test_create_target_registers_nothing_when_response_mapping_fails(self, sqlite_instance) -> None:
+        service = TargetService()
+
+        with (
+            patch(
+                "pyrit.backend.services.target_service.target_object_to_instance",
+                side_effect=RuntimeError("mapping failed"),
+            ),
+            pytest.raises(RuntimeError, match="mapping failed"),
+        ):
+            await service.create_target_async(
+                request=CreateTargetRequest(name="unmapped", type="TextTarget", params={}),
+            )
+
+        assert service.get_target_object(target_registry_name="unmapped") is None
+
     async def test_create_target_delegates_construction_to_registry(self, sqlite_instance) -> None:
         """Every target construction path is owned by the registry."""
         service = TargetService()