refactor(orchestrator): swap SubprocessBottleRunner → ProgrammaticBottleRunner
BottleRunner Protocol tightened: start() → str, freeze/resume/destroy → None. RunResult removed. lifecycle.py unpacks the slug directly. FakeRunner and test_runner updated to match. Config.bot_bottle_cli dropped (nothing uses it). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
This commit is contained in:
@@ -7,7 +7,7 @@ from __future__ import annotations
|
||||
|
||||
from collections.abc import Sequence
|
||||
|
||||
from bot_bottle.orchestrator.runner import RunResult, slugify
|
||||
from bot_bottle.orchestrator.runner import slugify
|
||||
|
||||
|
||||
class FakeForge:
|
||||
@@ -52,18 +52,15 @@ class FakeRunner:
|
||||
label: str,
|
||||
prompt: str,
|
||||
forge_env: dict[str, str],
|
||||
) -> RunResult:
|
||||
) -> str:
|
||||
self.calls.append(("start", agent, tuple(bottles), label, prompt, dict(forge_env)))
|
||||
return RunResult(slug=slugify(label), exit_code=0)
|
||||
return slugify(label)
|
||||
|
||||
def freeze(self, slug: str) -> int:
|
||||
def freeze(self, slug: str) -> None:
|
||||
self.calls.append(("freeze", slug))
|
||||
return 0
|
||||
|
||||
def resume(self, slug: str, prompt: str) -> RunResult:
|
||||
def resume(self, slug: str, prompt: str) -> None:
|
||||
self.calls.append(("resume", slug, prompt))
|
||||
return RunResult(slug=slug, exit_code=0)
|
||||
|
||||
def destroy(self, slug: str) -> int:
|
||||
def destroy(self, slug: str) -> None:
|
||||
self.calls.append(("destroy", slug))
|
||||
return 0
|
||||
|
||||
@@ -28,7 +28,6 @@ def _config(tmp: str) -> Config:
|
||||
watchdog_timeout_secs=1800,
|
||||
webhook_host="127.0.0.1",
|
||||
webhook_port=0,
|
||||
bot_bottle_cli="cli.py",
|
||||
queue_dir=Path(tmp) / "q",
|
||||
sidecar_socket=Path(tmp) / "s.sock",
|
||||
db_path=None,
|
||||
|
||||
@@ -1,11 +1,13 @@
|
||||
"""Unit: SubprocessBottleRunner + slugify (injected run fn)."""
|
||||
"""Unit: ProgrammaticBottleRunner + slugify."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import sys
|
||||
import types
|
||||
import unittest
|
||||
from collections.abc import Sequence
|
||||
from unittest.mock import MagicMock
|
||||
|
||||
from bot_bottle.orchestrator.runner import SubprocessBottleRunner, slugify
|
||||
from bot_bottle.orchestrator.runner import ProgrammaticBottleRunner, slugify
|
||||
|
||||
|
||||
class SlugifyTest(unittest.TestCase):
|
||||
@@ -17,64 +19,75 @@ class SlugifyTest(unittest.TestCase):
|
||||
self.assertEqual("a-b-c", slugify(" A_B/C!! "))
|
||||
|
||||
|
||||
class SubprocessRunnerTest(unittest.TestCase):
|
||||
def _make_api_stub(**overrides):
|
||||
"""Return a mock bot_bottle.api module with sensible defaults."""
|
||||
stub = types.ModuleType("bot_bottle.api")
|
||||
stub.start_headless = MagicMock(return_value="impl-r-17")
|
||||
stub.freeze = MagicMock()
|
||||
stub.resume_headless = MagicMock()
|
||||
stub.destroy = MagicMock()
|
||||
for k, v in overrides.items():
|
||||
setattr(stub, k, v)
|
||||
return stub
|
||||
|
||||
|
||||
class ProgrammaticRunnerTest(unittest.TestCase):
|
||||
def setUp(self):
|
||||
self.argvs: list[list[str]] = []
|
||||
self.envs: list[dict[str, str]] = []
|
||||
self._api = _make_api_stub()
|
||||
sys.modules["bot_bottle.api"] = self._api
|
||||
self.runner = ProgrammaticBottleRunner()
|
||||
|
||||
def fake_run(argv: Sequence[str], env: dict[str, str]) -> int:
|
||||
self.argvs.append(list(argv))
|
||||
self.envs.append(dict(env))
|
||||
return 0
|
||||
def tearDown(self):
|
||||
sys.modules.pop("bot_bottle.api", None)
|
||||
|
||||
self.runner = SubprocessBottleRunner(
|
||||
cli="/x/cli.py", base_env={"PATH": "/bin"}, python="/py", run=fake_run
|
||||
)
|
||||
|
||||
def test_start_argv_and_env(self):
|
||||
result = self.runner.start(
|
||||
def test_start_returns_slug_from_api(self):
|
||||
slug = self.runner.start(
|
||||
agent="impl", bottles=["claude", "dev"], label="impl-r-17",
|
||||
prompt="do it", forge_env={"FORGE_OWNER": "didericis"},
|
||||
)
|
||||
self.assertEqual("impl-r-17", result.slug)
|
||||
argv = self.argvs[0]
|
||||
self.assertEqual(["/py", "/x/cli.py", "start", "impl", "--headless",
|
||||
"--label", "impl-r-17", "--prompt", "do it",
|
||||
"--bottle", "claude", "--bottle", "dev"], argv)
|
||||
# forge_env merged over base_env for the child.
|
||||
self.assertEqual("didericis", self.envs[0]["FORGE_OWNER"])
|
||||
self.assertEqual("/bin", self.envs[0]["PATH"])
|
||||
self.assertEqual("impl-r-17", slug)
|
||||
|
||||
def test_start_no_bottles_omits_flag(self):
|
||||
def test_start_forwards_all_args(self):
|
||||
self.runner.start(
|
||||
agent="impl", bottles=["claude", "dev"], label="impl-r-17",
|
||||
prompt="do it", forge_env={"FORGE_OWNER": "didericis"},
|
||||
)
|
||||
self._api.start_headless.assert_called_once_with(
|
||||
"impl",
|
||||
prompt="do it",
|
||||
bottles=["claude", "dev"],
|
||||
label="impl-r-17",
|
||||
forge_env={"FORGE_OWNER": "didericis"},
|
||||
)
|
||||
|
||||
def test_start_no_bottles_passes_none(self):
|
||||
self.runner.start(agent="impl", bottles=[], label="l", prompt="p", forge_env={})
|
||||
self.assertNotIn("--bottle", self.argvs[0])
|
||||
call_kwargs = self._api.start_headless.call_args[1]
|
||||
self.assertIsNone(call_kwargs["bottles"])
|
||||
|
||||
def test_freeze_calls_commit(self):
|
||||
def test_freeze_delegates_to_api(self):
|
||||
self.runner.freeze("slug-1")
|
||||
self.assertEqual(["/py", "/x/cli.py", "commit", "slug-1"], self.argvs[0])
|
||||
self._api.freeze.assert_called_once_with("slug-1")
|
||||
|
||||
def test_resume_headless(self):
|
||||
r = self.runner.resume("slug-1", "address review")
|
||||
self.assertEqual("slug-1", r.slug)
|
||||
self.assertEqual(
|
||||
["/py", "/x/cli.py", "resume", "slug-1", "--headless", "--prompt",
|
||||
"address review"], self.argvs[0])
|
||||
def test_freeze_returns_none(self):
|
||||
result = self.runner.freeze("slug-1")
|
||||
self.assertIsNone(result)
|
||||
|
||||
def test_destroy_calls_cleanup(self):
|
||||
code = self.runner.destroy("slug-7")
|
||||
self.assertEqual(0, code)
|
||||
self.assertEqual(["/py", "/x/cli.py", "cleanup", "slug-7"], self.argvs[0])
|
||||
def test_resume_delegates_to_api(self):
|
||||
self.runner.resume("slug-1", "address review")
|
||||
self._api.resume_headless.assert_called_once_with("slug-1", prompt="address review")
|
||||
|
||||
def test_resume_returns_none(self):
|
||||
result = self.runner.resume("slug-1", "p")
|
||||
self.assertIsNone(result)
|
||||
|
||||
class DefaultRunTest(unittest.TestCase):
|
||||
def test_calls_subprocess_and_returns_code(self):
|
||||
from unittest.mock import MagicMock, patch
|
||||
from bot_bottle.orchestrator.runner import _default_run
|
||||
with patch("subprocess.run") as mock_run:
|
||||
mock_run.return_value = MagicMock(returncode=42)
|
||||
code = _default_run(["echo", "hi"], {"PATH": "/bin"})
|
||||
self.assertEqual(42, code)
|
||||
mock_run.assert_called_once_with(["echo", "hi"], env={"PATH": "/bin"}, check=False)
|
||||
def test_destroy_delegates_to_api(self):
|
||||
self.runner.destroy("slug-7")
|
||||
self._api.destroy.assert_called_once_with("slug-7")
|
||||
|
||||
def test_destroy_returns_none(self):
|
||||
result = self.runner.destroy("slug-7")
|
||||
self.assertIsNone(result)
|
||||
|
||||
|
||||
if __name__ == "__main__":
|
||||
|
||||
Reference in New Issue
Block a user