fix: resolve teardown timeout before registration to prevent orphaned state

If resolve_teardown_timeout() raised after launch_consolidated()/register_agent()
returned, the teardown callback was never registered, leaving the bottle
registered with no cleanup path. Resolve the timeout into a local variable
before the registration call so any failure aborts before the bottle exists.

Add tests covering OrchestratorConfigStore, resolve_teardown_timeout priority
ordering (env > db > default), and the source-order invariant that the resolver
runs before registration in all three backends.

tests/unit/test_orchestrator_config_store.py

@@ -0,0 +1,149 @@
+"""Unit: OrchestratorConfigStore and resolve_teardown_timeout.
+
+Also verifies the lifecycle ordering invariant: resolve_teardown_timeout()
+must be called before launch_consolidated() / register_agent() so that a
+resolver failure cannot leave an orphaned registration with no teardown
+callback.
+"""
+
+from __future__ import annotations
+
+import inspect
+import os
+import tempfile
+import unittest
+from pathlib import Path
+from unittest.mock import patch
+
+from bot_bottle.orchestrator.config_store import (
+    DEFAULT_TEARDOWN_TIMEOUT_SECONDS,
+    TEARDOWN_TIMEOUT_ENV,
+    TEARDOWN_TIMEOUT_KEY,
+    OrchestratorConfigStore,
+    resolve_teardown_timeout,
+)
+
+
+class TestOrchestratorConfigStore(unittest.TestCase):
+    def setUp(self) -> None:
+        self._tmp = tempfile.TemporaryDirectory()
+        self.db = Path(self._tmp.name) / "test.db"
+        self.store = OrchestratorConfigStore(self.db)
+        self.store.migrate()
+
+    def tearDown(self) -> None:
+        self._tmp.cleanup()
+
+    def test_get_returns_none_for_absent_key(self) -> None:
+        self.assertIsNone(self.store.get("missing"))
+
+    def test_set_and_get_roundtrip(self) -> None:
+        self.store.set("foo", "bar")
+        self.assertEqual("bar", self.store.get("foo"))
+
+    def test_set_upserts_existing_key(self) -> None:
+        self.store.set("k", "v1")
+        self.store.set("k", "v2")
+        self.assertEqual("v2", self.store.get("k"))
+
+    def test_delete_removes_key(self) -> None:
+        self.store.set("k", "v")
+        deleted = self.store.delete("k")
+        self.assertTrue(deleted)
+        self.assertIsNone(self.store.get("k"))
+
+    def test_delete_absent_returns_false(self) -> None:
+        self.assertFalse(self.store.delete("nope"))
+
+    def test_is_migrated_true_after_migrate(self) -> None:
+        self.assertTrue(self.store.is_migrated())
+
+    def test_is_migrated_false_before_migrate(self) -> None:
+        store = OrchestratorConfigStore(Path(self._tmp.name) / "new.db")
+        self.assertFalse(store.is_migrated())
+
+
+class TestResolveTeardownTimeout(unittest.TestCase):
+    def setUp(self) -> None:
+        self._tmp = tempfile.TemporaryDirectory()
+        self.db = Path(self._tmp.name) / "cfg.db"
+
+    def tearDown(self) -> None:
+        self._tmp.cleanup()
+        os.environ.pop(TEARDOWN_TIMEOUT_ENV, None)
+
+    def test_returns_default_when_nothing_configured(self) -> None:
+        self.assertEqual(
+            DEFAULT_TEARDOWN_TIMEOUT_SECONDS,
+            resolve_teardown_timeout(self.db),
+        )
+
+    def test_env_var_overrides_default(self) -> None:
+        os.environ[TEARDOWN_TIMEOUT_ENV] = "99"
+        self.assertEqual(99.0, resolve_teardown_timeout(self.db))
+
+    def test_env_var_overrides_db_value(self) -> None:
+        store = OrchestratorConfigStore(self.db)
+        store.migrate()
+        store.set(TEARDOWN_TIMEOUT_KEY, "55")
+        os.environ[TEARDOWN_TIMEOUT_ENV] = "77"
+        self.assertEqual(77.0, resolve_teardown_timeout(self.db))
+
+    def test_db_value_overrides_default(self) -> None:
+        store = OrchestratorConfigStore(self.db)
+        store.migrate()
+        store.set(TEARDOWN_TIMEOUT_KEY, "42")
+        self.assertEqual(42.0, resolve_teardown_timeout(self.db))
+
+    def test_invalid_env_var_falls_through_to_default(self) -> None:
+        os.environ[TEARDOWN_TIMEOUT_ENV] = "not-a-number"
+        self.assertEqual(DEFAULT_TEARDOWN_TIMEOUT_SECONDS, resolve_teardown_timeout(self.db))
+
+    def test_non_positive_env_var_falls_through_to_default(self) -> None:
+        os.environ[TEARDOWN_TIMEOUT_ENV] = "0"
+        self.assertEqual(DEFAULT_TEARDOWN_TIMEOUT_SECONDS, resolve_teardown_timeout(self.db))
+
+    def test_invalid_db_value_falls_through_to_default(self) -> None:
+        store = OrchestratorConfigStore(self.db)
+        store.migrate()
+        store.set(TEARDOWN_TIMEOUT_KEY, "bad")
+        self.assertEqual(DEFAULT_TEARDOWN_TIMEOUT_SECONDS, resolve_teardown_timeout(self.db))
+
+    def test_migrates_db_on_first_call(self) -> None:
+        # DB does not exist yet; resolve_teardown_timeout must not raise.
+        result = resolve_teardown_timeout(self.db)
+        self.assertEqual(DEFAULT_TEARDOWN_TIMEOUT_SECONDS, result)
+        self.assertTrue(OrchestratorConfigStore(self.db).is_migrated())
+
+
+class TestTeardownTimeoutResolvedBeforeRegistration(unittest.TestCase):
+    """Ordering invariant: if resolve_teardown_timeout() raises, the bottle
+    must not yet be registered — no orphaned state can result."""
+
+    def _src(self, module) -> str:
+        return inspect.getsource(module)
+
+    def test_docker_resolves_timeout_before_launch_consolidated(self) -> None:
+        from bot_bottle.backend.docker import launch
+        src = self._src(launch)
+        resolve_at = src.index("teardown_timeout = resolve_teardown_timeout()")
+        launch_at = src.index("ctx = launch_consolidated(")
+        self.assertLess(resolve_at, launch_at)
+
+    def test_firecracker_resolves_timeout_before_launch_consolidated(self) -> None:
+        from bot_bottle.backend.firecracker import launch
+        src = self._src(launch)
+        resolve_at = src.index("teardown_timeout = resolve_teardown_timeout()")
+        launch_at = src.index("ctx = launch_consolidated(")
+        self.assertLess(resolve_at, launch_at)
+
+    def test_macos_resolves_timeout_before_register_agent(self) -> None:
+        from bot_bottle.backend.macos_container import launch
+        src = self._src(launch)
+        resolve_at = src.index("teardown_timeout = resolve_teardown_timeout()")
+        register_at = src.index("ctx = register_agent(")
+        self.assertLess(resolve_at, register_at)
+
+
+if __name__ == "__main__":
+    unittest.main()