From 49739980db3ff29f29ba95b532a735d119330508 Mon Sep 17 00:00:00 2001
From: "jinli.yl" <jinli.yl@alibaba-inc.com>
Date: Fri, 28 Aug 2026 14:57:39 +0800
Subject: [PATCH] fix(core): roll back partial component startup

---
 reme/application.py               | 11 +------
 reme/components/base_component.py | 38 ++++++++++++++++++++----
 tests/unit/test_base_component.py | 49 +++++++++++++++++++++++++++++++
 3 files changed, 82 insertions(+), 16 deletions(-)

diff --git a/reme/application.py b/reme/application.py
index 63b41240..b4b0cb69 100644
--- a/reme/application.py
+++ b/reme/application.py
@@ -345,16 +345,7 @@ class Application(BaseComponent):
             )
             if was_started:
                 await old_component.dump()
-                try:
-                    await replacement.start()
-                except BaseException:
-                    try:
-                        await replacement.close()
-                    except BaseException as cleanup_error:
-                        self.logger.exception(
-                            f"Failed to clean up replacement {component_type}:{name}: {cleanup_error}",
-                        )
-                    raise
+                await replacement.start()
 
             # Commit the new generation synchronously so observers cannot see
             # a context with only some dependency references updated.
diff --git a/reme/components/base_component.py b/reme/components/base_component.py
index 303af29a..74ef4633 100644
--- a/reme/components/base_component.py
+++ b/reme/components/base_component.py
@@ -223,15 +223,41 @@ class BaseComponent(ComponentMixin, ABC):
     # ----- Lifecycle control --------------------------------------------
 
     async def start(self) -> None:
-        """Start the component once: resolve deps → start owned → run _start."""
+        """Start once, rolling back partial resources when any startup stage fails."""
         async with self._lock:
             if self._is_started:
                 return
-            await self._resolve_bindings()
-            for owned in self._owned:
-                await owned.start()
-            await self._start()
-            self._is_started = True
+            started_owned: list[BaseComponent] = []
+            start_hook_entered = False
+            try:
+                await self._resolve_bindings()
+                for owned in self._owned:
+                    await owned.start()
+                    started_owned.append(owned)
+                start_hook_entered = True
+                await self._start()
+                self._is_started = True
+            except BaseException:
+                await self._rollback_start(start_hook_entered, started_owned)
+                raise
+
+    async def _rollback_start(
+        self,
+        start_hook_entered: bool,
+        started_owned: list["BaseComponent"],
+    ) -> None:
+        """Best-effort cleanup that preserves the original startup error."""
+        if start_hook_entered:
+            try:
+                await self._close()
+            except BaseException as exc:
+                self.logger.exception(f"Failed to roll back partially started component {self.name}: {exc}")
+        for owned in reversed(started_owned):
+            try:
+                await owned.close()
+            except BaseException as exc:
+                self.logger.exception(f"Failed to close owned component {owned.name} during startup rollback: {exc}")
+        self._is_started = False
 
     async def close(self) -> None:
         """Close the component once: run _close → close owned in reverse order."""
diff --git a/tests/unit/test_base_component.py b/tests/unit/test_base_component.py
index 5636ab74..df7e8ead 100644
--- a/tests/unit/test_base_component.py
+++ b/tests/unit/test_base_component.py
@@ -3,6 +3,7 @@
 # pylint: disable=protected-access,missing-function-docstring,missing-class-docstring,attribute-defined-outside-init
 
 import asyncio
+import contextlib
 import os
 import tempfile
 
@@ -205,6 +206,54 @@ def test_close_closes_owned_when_parent_close_fails():
     asyncio.run(run())
 
 
+def test_start_failure_rolls_back_partial_resources():
+    class PartialStartComponent(BaseComponent):
+        component_type = ComponentEnum.FILE_CHUNKER
+
+        def __init__(self):
+            super().__init__()
+            self.resource_open = False
+            self.task = None
+            self.rollback_count = 0
+
+        async def _start(self):
+            self.resource_open = True
+            self.task = asyncio.create_task(asyncio.Event().wait())
+            raise RuntimeError("partial startup failed")
+
+        async def _close(self):
+            self.rollback_count += 1
+            self.resource_open = False
+            if self.task is not None:
+                self.task.cancel()
+                with contextlib.suppress(asyncio.CancelledError):
+                    await self.task
+                self.task = None
+
+    async def run():
+        comp = PartialStartComponent()
+        owned = StubComponent(name="owned")
+        comp.dep = BaseComponent.bind(
+            "owned",
+            StubComponent,
+            default_factory=lambda: owned,
+        )
+
+        with pytest.raises(RuntimeError, match="partial startup failed"):
+            await comp.start()
+
+        assert comp.is_started is False
+        assert comp.resource_open is False
+        assert comp.task is None
+        assert comp.rollback_count == 1
+        assert owned.is_started is False
+        assert owned.close_count == 1
+        await comp.close()
+        assert comp.rollback_count == 1
+
+    asyncio.run(run())
+
+
 # -- standalone resolution ----------------------------------------------------