Compare commits
2 Commits
| Author | SHA1 | Date | |
|---|---|---|---|
| 18e610c7a8 | |||
| d5fb159857 |
@@ -22,7 +22,7 @@ from ..contrib.gitea.forge_state import ForgeState, SqliteForgeStateStore
|
|||||||
from .config import Config
|
from .config import Config
|
||||||
from .lifecycle import Orchestrator
|
from .lifecycle import Orchestrator
|
||||||
from .model import RunRecord
|
from .model import RunRecord
|
||||||
from .runner import SubprocessBottleRunner
|
from .runner import ProgrammaticBottleRunner
|
||||||
from .sidecar import ForgeSidecar, OpLog, drain_done_events
|
from .sidecar import ForgeSidecar, OpLog, drain_done_events
|
||||||
from .watchdog import Watchdog
|
from .watchdog import Watchdog
|
||||||
from .webhook import WebhookServer
|
from .webhook import WebhookServer
|
||||||
@@ -104,7 +104,7 @@ def make_sidecar(
|
|||||||
|
|
||||||
def build(config: Config) -> tuple[WebhookServer, Watchdog, Orchestrator]:
|
def build(config: Config) -> tuple[WebhookServer, Watchdog, Orchestrator]:
|
||||||
store = BotBottleStateStore(config.db_path)
|
store = BotBottleStateStore(config.db_path)
|
||||||
runner = SubprocessBottleRunner(cli=config.bot_bottle_cli, base_env=dict(os.environ))
|
runner = ProgrammaticBottleRunner()
|
||||||
membership_forge = make_forge(config, "_", "_")
|
membership_forge = make_forge(config, "_", "_")
|
||||||
orchestrator = Orchestrator(
|
orchestrator = Orchestrator(
|
||||||
forge=membership_forge,
|
forge=membership_forge,
|
||||||
|
|||||||
@@ -26,7 +26,6 @@ class Config:
|
|||||||
watchdog_timeout_secs: int
|
watchdog_timeout_secs: int
|
||||||
webhook_host: str
|
webhook_host: str
|
||||||
webhook_port: int
|
webhook_port: int
|
||||||
bot_bottle_cli: str
|
|
||||||
queue_dir: Path
|
queue_dir: Path
|
||||||
sidecar_socket: Path
|
sidecar_socket: Path
|
||||||
db_path: Path | None
|
db_path: Path | None
|
||||||
@@ -43,7 +42,6 @@ class Config:
|
|||||||
watchdog_timeout_secs=int(e.get("FORGE_WATCHDOG_TIMEOUT", "1800")),
|
watchdog_timeout_secs=int(e.get("FORGE_WATCHDOG_TIMEOUT", "1800")),
|
||||||
webhook_host=e.get("FORGE_WEBHOOK_HOST", "127.0.0.1"),
|
webhook_host=e.get("FORGE_WEBHOOK_HOST", "127.0.0.1"),
|
||||||
webhook_port=int(e.get("FORGE_WEBHOOK_PORT", "8477")),
|
webhook_port=int(e.get("FORGE_WEBHOOK_PORT", "8477")),
|
||||||
bot_bottle_cli=e.get("BOT_BOTTLE_CLI", "cli.py"),
|
|
||||||
queue_dir=Path(e.get("FORGE_QUEUE_DIR", str(default_root / "forge-queue"))),
|
queue_dir=Path(e.get("FORGE_QUEUE_DIR", str(default_root / "forge-queue"))),
|
||||||
sidecar_socket=Path(
|
sidecar_socket=Path(
|
||||||
e.get("FORGE_SIDECAR_SOCKET", str(default_root / "forge-sidecar.sock"))
|
e.get("FORGE_SIDECAR_SOCKET", str(default_root / "forge-sidecar.sock"))
|
||||||
|
|||||||
@@ -110,7 +110,7 @@ class Orchestrator:
|
|||||||
def _launch(self, event: IssueAssigned, target: Target) -> None:
|
def _launch(self, event: IssueAssigned, target: Target) -> None:
|
||||||
label = self._label_for(target.agent_name, event)
|
label = self._label_for(target.agent_name, event)
|
||||||
bottles = [target.bottle_override] if target.bottle_override else []
|
bottles = [target.bottle_override] if target.bottle_override else []
|
||||||
result = self._runner.start(
|
slug = self._runner.start(
|
||||||
agent=target.agent_name,
|
agent=target.agent_name,
|
||||||
bottles=bottles,
|
bottles=bottles,
|
||||||
label=label,
|
label=label,
|
||||||
@@ -122,7 +122,7 @@ class Orchestrator:
|
|||||||
owner=event.owner,
|
owner=event.owner,
|
||||||
repo=event.repo,
|
repo=event.repo,
|
||||||
issue_number=event.issue_number,
|
issue_number=event.issue_number,
|
||||||
slug=result.slug,
|
slug=slug,
|
||||||
agent_name=target.agent_name,
|
agent_name=target.agent_name,
|
||||||
bottle_names=bottles,
|
bottle_names=bottles,
|
||||||
status=STATUS_RUNNING,
|
status=STATUS_RUNNING,
|
||||||
|
|||||||
@@ -1,31 +1,22 @@
|
|||||||
"""Bottle runner: drive the bot-bottle CLI to manage a bottle's life.
|
"""Bottle runner: drive bot_bottle to manage a bottle's life.
|
||||||
|
|
||||||
`BottleRunner` is the interface the lifecycle depends on;
|
`BottleRunner` is the interface the lifecycle depends on;
|
||||||
`SubprocessBottleRunner` shells out to the bot-bottle `cli.py`
|
`ProgrammaticBottleRunner` calls into the bot_bottle Python API directly
|
||||||
(`start --headless`, `commit`, `resume --headless`). The subprocess
|
(no subprocess). The slug returned by `start` is the actual slug minted
|
||||||
callable is injectable so tests never spawn a process.
|
at launch time — not a post-hoc derivation from the label — so it is
|
||||||
|
authoritative even if bot-bottle's slugification logic changes.
|
||||||
|
|
||||||
The slug is derived from the label via `slugify`, matching bot-bottle's
|
`slugify` is retained for `FakeRunner` (tests) and for the label scheme
|
||||||
container-slug rule; the orchestrator picks labels that embed the issue
|
the orchestrator uses to predict collision-free slugs.
|
||||||
identity so slugs are unique and collisions never rename them.
|
|
||||||
"""
|
"""
|
||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
import re
|
import re
|
||||||
import subprocess
|
from collections.abc import Sequence
|
||||||
import sys
|
|
||||||
from collections.abc import Callable, Sequence
|
|
||||||
from dataclasses import dataclass
|
|
||||||
from typing import Protocol
|
from typing import Protocol
|
||||||
|
|
||||||
|
|
||||||
@dataclass(frozen=True)
|
|
||||||
class RunResult:
|
|
||||||
slug: str
|
|
||||||
exit_code: int
|
|
||||||
|
|
||||||
|
|
||||||
class BottleRunner(Protocol):
|
class BottleRunner(Protocol):
|
||||||
def start(
|
def start(
|
||||||
self,
|
self,
|
||||||
@@ -35,13 +26,13 @@ class BottleRunner(Protocol):
|
|||||||
label: str,
|
label: str,
|
||||||
prompt: str,
|
prompt: str,
|
||||||
forge_env: dict[str, str],
|
forge_env: dict[str, str],
|
||||||
) -> RunResult: ...
|
) -> str: ...
|
||||||
|
|
||||||
def freeze(self, slug: str) -> int: ...
|
def freeze(self, slug: str) -> None: ...
|
||||||
|
|
||||||
def resume(self, slug: str, prompt: str) -> RunResult: ...
|
def resume(self, slug: str, prompt: str) -> None: ...
|
||||||
|
|
||||||
def destroy(self, slug: str) -> int: ...
|
def destroy(self, slug: str) -> None: ...
|
||||||
|
|
||||||
|
|
||||||
_SLUG_RE = re.compile(r"[^a-z0-9]+")
|
_SLUG_RE = re.compile(r"[^a-z0-9]+")
|
||||||
@@ -53,34 +44,13 @@ def slugify(label: str) -> str:
|
|||||||
return _SLUG_RE.sub("-", label.lower()).strip("-")
|
return _SLUG_RE.sub("-", label.lower()).strip("-")
|
||||||
|
|
||||||
|
|
||||||
# A subprocess.run-shaped callable, injectable for tests.
|
class ProgrammaticBottleRunner:
|
||||||
RunFn = Callable[[Sequence[str], dict[str, str]], int]
|
"""Calls into the bot_bottle Python API directly — no subprocess.
|
||||||
|
|
||||||
|
Imports are deferred to call time so tests can inject a mock into
|
||||||
def _default_run(argv: Sequence[str], env: dict[str, str]) -> int:
|
sys.modules['bot_bottle.api'] before calling runner methods.
|
||||||
return subprocess.run(list(argv), env=env, check=False).returncode
|
bot_bottle.api is added in the forge-native-integration PR (#318),
|
||||||
|
which merges before this one."""
|
||||||
|
|
||||||
class SubprocessBottleRunner:
|
|
||||||
"""Shells the bot-bottle CLI. `cli` is the path to `cli.py`; `python`
|
|
||||||
is the interpreter to run it with; `base_env` is the environment the
|
|
||||||
child inherits (the orchestrator's, minus per-run additions)."""
|
|
||||||
|
|
||||||
def __init__(
|
|
||||||
self,
|
|
||||||
*,
|
|
||||||
cli: str,
|
|
||||||
base_env: dict[str, str],
|
|
||||||
python: str = sys.executable,
|
|
||||||
run: RunFn = _default_run,
|
|
||||||
) -> None:
|
|
||||||
self._cli = cli
|
|
||||||
self._python = python
|
|
||||||
self._base_env = base_env
|
|
||||||
self._run = run
|
|
||||||
|
|
||||||
def _argv(self, *args: str) -> list[str]:
|
|
||||||
return [self._python, self._cli, *args]
|
|
||||||
|
|
||||||
def start(
|
def start(
|
||||||
self,
|
self,
|
||||||
@@ -90,29 +60,24 @@ class SubprocessBottleRunner:
|
|||||||
label: str,
|
label: str,
|
||||||
prompt: str,
|
prompt: str,
|
||||||
forge_env: dict[str, str],
|
forge_env: dict[str, str],
|
||||||
) -> RunResult:
|
) -> str:
|
||||||
argv = self._argv(
|
from bot_bottle import api # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module
|
||||||
"start", agent, "--headless", "--label", label, "--prompt", prompt
|
return api.start_headless(
|
||||||
|
agent,
|
||||||
|
prompt=prompt,
|
||||||
|
bottles=list(bottles) or None,
|
||||||
|
label=label,
|
||||||
|
forge_env=forge_env,
|
||||||
)
|
)
|
||||||
for bottle in bottles:
|
|
||||||
argv += ["--bottle", bottle]
|
|
||||||
code = self._run(argv, {**self._base_env, **forge_env})
|
|
||||||
return RunResult(slug=slugify(label), exit_code=code)
|
|
||||||
|
|
||||||
def freeze(self, slug: str) -> int:
|
def freeze(self, slug: str) -> None:
|
||||||
# bot-bottle's `commit` snapshots a running bottle's state.
|
from bot_bottle import api # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module
|
||||||
return self._run(self._argv("commit", slug), self._base_env)
|
api.freeze(slug)
|
||||||
|
|
||||||
def resume(self, slug: str, prompt: str) -> RunResult:
|
def resume(self, slug: str, prompt: str) -> None:
|
||||||
code = self._run(
|
from bot_bottle import api # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module
|
||||||
self._argv("resume", slug, "--headless", "--prompt", prompt),
|
api.resume_headless(slug, prompt=prompt)
|
||||||
self._base_env,
|
|
||||||
)
|
|
||||||
return RunResult(slug=slug, exit_code=code)
|
|
||||||
|
|
||||||
def destroy(self, slug: str) -> int:
|
def destroy(self, slug: str) -> None:
|
||||||
# NOTE: bot-bottle `cleanup` currently targets all bottles; a
|
from bot_bottle import api # type: ignore[import-not-found] # pylint: disable=import-error,no-name-in-module
|
||||||
# per-slug teardown command is a known integration follow-up
|
api.destroy(slug)
|
||||||
# (tracked in docs/JOURNAL.md). Kept behind this method so the
|
|
||||||
# call site does not change when that lands.
|
|
||||||
return self._run(self._argv("cleanup", slug), self._base_env)
|
|
||||||
|
|||||||
@@ -106,7 +106,7 @@ class ForgeSidecar:
|
|||||||
def dispatch(self, method: str, params: dict[str, Any]) -> dict[str, Any]:
|
def dispatch(self, method: str, params: dict[str, Any]) -> dict[str, Any]:
|
||||||
try:
|
try:
|
||||||
result = self._invoke(method, params)
|
result = self._invoke(method, params)
|
||||||
except Exception as exc: # noqa: BLE001 — surface as JSON-RPC error
|
except Exception as exc: # noqa: BLE001 # pylint: disable=broad-exception-caught
|
||||||
self._log.record(method, params.get("number"), f"error: {exc}")
|
self._log.record(method, params.get("number"), f"error: {exc}")
|
||||||
return {"ok": False, "error": str(exc)}
|
return {"ok": False, "error": str(exc)}
|
||||||
return {"ok": True, "result": result}
|
return {"ok": True, "result": result}
|
||||||
|
|||||||
@@ -7,7 +7,7 @@ from __future__ import annotations
|
|||||||
|
|
||||||
from collections.abc import Sequence
|
from collections.abc import Sequence
|
||||||
|
|
||||||
from bot_bottle.orchestrator.runner import RunResult, slugify
|
from bot_bottle.orchestrator.runner import slugify
|
||||||
|
|
||||||
|
|
||||||
class FakeForge:
|
class FakeForge:
|
||||||
@@ -52,18 +52,15 @@ class FakeRunner:
|
|||||||
label: str,
|
label: str,
|
||||||
prompt: str,
|
prompt: str,
|
||||||
forge_env: dict[str, str],
|
forge_env: dict[str, str],
|
||||||
) -> RunResult:
|
) -> str:
|
||||||
self.calls.append(("start", agent, tuple(bottles), label, prompt, dict(forge_env)))
|
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))
|
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))
|
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))
|
self.calls.append(("destroy", slug))
|
||||||
return 0
|
|
||||||
|
|||||||
@@ -28,7 +28,6 @@ def _config(tmp: str) -> Config:
|
|||||||
watchdog_timeout_secs=1800,
|
watchdog_timeout_secs=1800,
|
||||||
webhook_host="127.0.0.1",
|
webhook_host="127.0.0.1",
|
||||||
webhook_port=0,
|
webhook_port=0,
|
||||||
bot_bottle_cli="cli.py",
|
|
||||||
queue_dir=Path(tmp) / "q",
|
queue_dir=Path(tmp) / "q",
|
||||||
sidecar_socket=Path(tmp) / "s.sock",
|
sidecar_socket=Path(tmp) / "s.sock",
|
||||||
db_path=None,
|
db_path=None,
|
||||||
|
|||||||
@@ -1,11 +1,14 @@
|
|||||||
"""Unit: SubprocessBottleRunner + slugify (injected run fn)."""
|
"""Unit: ProgrammaticBottleRunner + slugify."""
|
||||||
|
|
||||||
from __future__ import annotations
|
from __future__ import annotations
|
||||||
|
|
||||||
|
import sys
|
||||||
|
import types
|
||||||
import unittest
|
import unittest
|
||||||
from collections.abc import Sequence
|
from typing import Any
|
||||||
|
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):
|
class SlugifyTest(unittest.TestCase):
|
||||||
@@ -17,64 +20,75 @@ class SlugifyTest(unittest.TestCase):
|
|||||||
self.assertEqual("a-b-c", slugify(" A_B/C!! "))
|
self.assertEqual("a-b-c", slugify(" A_B/C!! "))
|
||||||
|
|
||||||
|
|
||||||
class SubprocessRunnerTest(unittest.TestCase):
|
def _make_api_stub(**overrides: object) -> Any:
|
||||||
def setUp(self):
|
"""Return a mock bot_bottle.api module with sensible defaults."""
|
||||||
self.argvs: list[list[str]] = []
|
stub: Any = types.ModuleType("bot_bottle.api")
|
||||||
self.envs: list[dict[str, str]] = []
|
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
|
||||||
|
|
||||||
def fake_run(argv: Sequence[str], env: dict[str, str]) -> int:
|
|
||||||
self.argvs.append(list(argv))
|
|
||||||
self.envs.append(dict(env))
|
|
||||||
return 0
|
|
||||||
|
|
||||||
self.runner = SubprocessBottleRunner(
|
class ProgrammaticRunnerTest(unittest.TestCase):
|
||||||
cli="/x/cli.py", base_env={"PATH": "/bin"}, python="/py", run=fake_run
|
def setUp(self) -> None:
|
||||||
)
|
self._api: Any = _make_api_stub()
|
||||||
|
sys.modules["bot_bottle.api"] = self._api
|
||||||
|
self.runner = ProgrammaticBottleRunner()
|
||||||
|
|
||||||
def test_start_argv_and_env(self):
|
def tearDown(self) -> None:
|
||||||
result = self.runner.start(
|
sys.modules.pop("bot_bottle.api", None)
|
||||||
|
|
||||||
|
def test_start_returns_slug_from_api(self) -> None:
|
||||||
|
slug = self.runner.start(
|
||||||
agent="impl", bottles=["claude", "dev"], label="impl-r-17",
|
agent="impl", bottles=["claude", "dev"], label="impl-r-17",
|
||||||
prompt="do it", forge_env={"FORGE_OWNER": "didericis"},
|
prompt="do it", forge_env={"FORGE_OWNER": "didericis"},
|
||||||
)
|
)
|
||||||
self.assertEqual("impl-r-17", result.slug)
|
self.assertEqual("impl-r-17", 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"])
|
|
||||||
|
|
||||||
def test_start_no_bottles_omits_flag(self):
|
def test_start_forwards_all_args(self) -> None:
|
||||||
|
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) -> None:
|
||||||
self.runner.start(agent="impl", bottles=[], label="l", prompt="p", forge_env={})
|
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) -> None:
|
||||||
self.runner.freeze("slug-1")
|
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):
|
def test_freeze_returns_none(self) -> None:
|
||||||
r = self.runner.resume("slug-1", "address review")
|
result = self.runner.freeze("slug-1")
|
||||||
self.assertEqual("slug-1", r.slug)
|
self.assertIsNone(result)
|
||||||
self.assertEqual(
|
|
||||||
["/py", "/x/cli.py", "resume", "slug-1", "--headless", "--prompt",
|
|
||||||
"address review"], self.argvs[0])
|
|
||||||
|
|
||||||
def test_destroy_calls_cleanup(self):
|
def test_resume_delegates_to_api(self) -> None:
|
||||||
code = self.runner.destroy("slug-7")
|
self.runner.resume("slug-1", "address review")
|
||||||
self.assertEqual(0, code)
|
self._api.resume_headless.assert_called_once_with("slug-1", prompt="address review")
|
||||||
self.assertEqual(["/py", "/x/cli.py", "cleanup", "slug-7"], self.argvs[0])
|
|
||||||
|
|
||||||
|
def test_resume_returns_none(self) -> None:
|
||||||
|
result = self.runner.resume("slug-1", "p")
|
||||||
|
self.assertIsNone(result)
|
||||||
|
|
||||||
class DefaultRunTest(unittest.TestCase):
|
def test_destroy_delegates_to_api(self) -> None:
|
||||||
def test_calls_subprocess_and_returns_code(self):
|
self.runner.destroy("slug-7")
|
||||||
from unittest.mock import MagicMock, patch
|
self._api.destroy.assert_called_once_with("slug-7")
|
||||||
from bot_bottle.orchestrator.runner import _default_run
|
|
||||||
with patch("subprocess.run") as mock_run:
|
def test_destroy_returns_none(self) -> None:
|
||||||
mock_run.return_value = MagicMock(returncode=42)
|
result = self.runner.destroy("slug-7")
|
||||||
code = _default_run(["echo", "hi"], {"PATH": "/bin"})
|
self.assertIsNone(result)
|
||||||
self.assertEqual(42, code)
|
|
||||||
mock_run.assert_called_once_with(["echo", "hi"], env={"PATH": "/bin"}, check=False)
|
|
||||||
|
|
||||||
|
|
||||||
if __name__ == "__main__":
|
if __name__ == "__main__":
|
||||||
|
|||||||
Reference in New Issue
Block a user