Add 'python/robomp/' from commit '553fd1cfcf59e4c501c54fc81bc083ffd2ca007b'
git-subtree-dir: python/robomp git-subtree-mainline:4f6e70f779git-subtree-split:553fd1cfcf
This commit is contained in:
@@ -0,0 +1,154 @@
|
||||
"""Common pytest fixtures."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from robomp.config import Settings, reset_settings_cache
|
||||
from robomp.dashboard import reset_index_cache, static_dir
|
||||
from robomp.db import Database, close_database
|
||||
|
||||
# Minimum HTML the dashboard handler needs to render: `<title>` plus a script
|
||||
# block carrying the `__ROBOMP_CONFIG__` sentinel. The real Vite-built bundle
|
||||
# adds JS/CSS asset links; tests only care about the rendering contract.
|
||||
_PLACEHOLDER_INDEX_HTML = (
|
||||
"<!doctype html>\n"
|
||||
'<html lang="en">\n'
|
||||
' <head><meta charset="utf-8"><title>robomp</title></head>\n'
|
||||
" <body>\n"
|
||||
' <div id="app"></div>\n'
|
||||
' <script id="robomp-config" type="application/json">__ROBOMP_CONFIG__</script>\n'
|
||||
" </body>\n"
|
||||
"</html>\n"
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True, scope="session")
|
||||
def _ensure_dashboard_bundle() -> None:
|
||||
"""Guarantee a renderable dashboard bundle for the whole session.
|
||||
|
||||
The real bundle is produced by `bun run web:build`; CI and fresh clones
|
||||
might not have run it yet. We only synthesise an `index.html` when one
|
||||
isn't already present, so a developer's locally-built bundle isn't
|
||||
clobbered by the test run.
|
||||
"""
|
||||
directory = static_dir()
|
||||
index = directory / "index.html"
|
||||
if not index.exists():
|
||||
index.write_text(_PLACEHOLDER_INDEX_HTML, encoding="utf-8")
|
||||
reset_index_cache()
|
||||
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _open_tmp_path_for_slot_traversal(tmp_path: Path) -> None:
|
||||
"""Grant traverse (`+x`) on tmp_path's root-owned ancestors so slot
|
||||
subprocesses can reach the workspace.
|
||||
|
||||
pytest's default ``tmp_path`` lives under ``/tmp/pytest-of-<user>/`` with
|
||||
mode ``0700``. On macOS dev that's irrelevant (no slot subprocess ever
|
||||
drops uid). On Linux+root the slot UID (e.g. 2001) is non-zero and
|
||||
every directory between ``/`` and the workspace needs at least the
|
||||
`o+x` bit or the slot's stat fails with EACCES. Adds `o+x` (NOT `o+r`)
|
||||
so directory contents stay private; only path-traversal is allowed.
|
||||
"""
|
||||
import os
|
||||
import platform
|
||||
import stat
|
||||
|
||||
if platform.system() != "Linux" or os.geteuid() != 0:
|
||||
return
|
||||
cursor = tmp_path.resolve()
|
||||
while cursor != cursor.parent:
|
||||
try:
|
||||
st = cursor.stat()
|
||||
except FileNotFoundError:
|
||||
break
|
||||
if not stat.S_ISDIR(st.st_mode):
|
||||
break
|
||||
if not (st.st_mode & 0o001):
|
||||
try:
|
||||
cursor.chmod(st.st_mode | 0o001)
|
||||
except PermissionError:
|
||||
break
|
||||
cursor = cursor.parent
|
||||
|
||||
def _baseline_env(tmp_path: Path) -> dict[str, str]:
|
||||
return {
|
||||
# Orchestrator-mode: no PAT in this container; talk to gh-proxy instead.
|
||||
"ROBOMP_GH_PROXY_URL": "http://gh-proxy.invalid:8081",
|
||||
"ROBOMP_GH_PROXY_HMAC_KEY": "test-hmac-key-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa",
|
||||
"GITHUB_WEBHOOK_SECRET": "test-webhook-secret",
|
||||
"ROBOMP_BOT_LOGIN": "robomp-bot",
|
||||
"ROBOMP_GIT_AUTHOR_NAME": "robomp-bot",
|
||||
"ROBOMP_GIT_AUTHOR_EMAIL": "robomp-bot@example.invalid",
|
||||
"ROBOMP_REPO_ALLOWLIST": "octo/widget",
|
||||
"ROBOMP_MODEL": "anthropic/claude-sonnet-4-5",
|
||||
"ROBOMP_THINKING": "high",
|
||||
"ROBOMP_WORKSPACE_ROOT": str(tmp_path / "workspaces"),
|
||||
"ROBOMP_SQLITE_PATH": str(tmp_path / "robomp.sqlite"),
|
||||
"ROBOMP_LOG_DIR": str(tmp_path / "logs"),
|
||||
# Production default is `/data/cache/pi-natives` (provisioned by the
|
||||
# container entrypoint). Tests need a writable, isolated path; we also
|
||||
# default-disable the cache so its background GC loop doesn't add
|
||||
# noise to event-dispatcher timing assertions. Tests that want the
|
||||
# cache flip `ROBOMP_NATIVES_CACHE_ENABLED=true` explicitly.
|
||||
"ROBOMP_NATIVES_CACHE_ROOT": str(tmp_path / "natives-cache"),
|
||||
"ROBOMP_NATIVES_CACHE_ENABLED": "false",
|
||||
}
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def env(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> dict[str, str]:
|
||||
env = _baseline_env(tmp_path)
|
||||
for key, value in env.items():
|
||||
monkeypatch.setenv(key, value)
|
||||
# Defensive: a stray `.env` or shell export must not flip us into PAT mode.
|
||||
# `monkeypatch.delenv` would let pydantic_settings fall back to the .env
|
||||
# file; setenv("") is what actually shadows the file value, and the
|
||||
# `_blank_token_disables` validator treats empty strings as unset.
|
||||
monkeypatch.setenv("GITHUB_TOKEN", "")
|
||||
monkeypatch.delenv("ROBOMP_PROVIDER", raising=False)
|
||||
monkeypatch.setenv("ROBOMP_REPLAY_TOKEN", "")
|
||||
reset_settings_cache()
|
||||
yield env
|
||||
reset_settings_cache()
|
||||
close_database()
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def proxy_env(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> dict[str, str]:
|
||||
"""Baseline env for the gh-proxy container: holds the PAT, no proxy vars."""
|
||||
baseline = _baseline_env(tmp_path)
|
||||
baseline.pop("ROBOMP_GH_PROXY_URL", None)
|
||||
baseline.pop("ROBOMP_GH_PROXY_HMAC_KEY", None)
|
||||
baseline["GITHUB_TOKEN"] = "ghp_test_token_value_xxxxxxxxxxxxxxxx"
|
||||
for key, value in baseline.items():
|
||||
monkeypatch.setenv(key, value)
|
||||
# Same defense-in-depth as `env`: setenv("") rather than delenv so
|
||||
# pydantic_settings doesn't fall back to the on-disk `.env` file.
|
||||
monkeypatch.setenv("ROBOMP_GH_PROXY_URL", "")
|
||||
monkeypatch.setenv("ROBOMP_GH_PROXY_HMAC_KEY", "")
|
||||
monkeypatch.delenv("ROBOMP_PROVIDER", raising=False)
|
||||
monkeypatch.setenv("ROBOMP_REPLAY_TOKEN", "")
|
||||
reset_settings_cache()
|
||||
yield baseline
|
||||
reset_settings_cache()
|
||||
close_database()
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def settings(env: dict[str, str]) -> Settings:
|
||||
cfg = Settings() # type: ignore[call-arg]
|
||||
cfg.ensure_paths()
|
||||
return cfg
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def db(tmp_path: Path) -> Database:
|
||||
path = tmp_path / "test.sqlite"
|
||||
database = Database(path)
|
||||
yield database
|
||||
database.close()
|
||||
@@ -0,0 +1,191 @@
|
||||
"""Coverage for `AutocloseScheduler` against in-process fakes."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from collections.abc import Iterable
|
||||
|
||||
import pytest
|
||||
from pydantic import SecretStr
|
||||
|
||||
from robomp.autoclose import AutocloseScheduler
|
||||
from robomp.config import Settings
|
||||
from robomp.db import Database, issue_key
|
||||
from robomp.github_client import GitHubError, ReactionInfo
|
||||
|
||||
|
||||
def _settings(*, enabled: bool = True, hours: float = 4.0, scan: float = 60.0) -> Settings:
|
||||
return Settings.model_construct(
|
||||
github_token=None,
|
||||
github_webhook_secret=SecretStr("x"),
|
||||
bot_login="robomp-bot",
|
||||
git_author_email="bot@example.invalid",
|
||||
repo_allowlist_raw="octo/widget",
|
||||
gh_proxy_url="http://proxy.invalid",
|
||||
gh_proxy_hmac_key=SecretStr("k" * 32),
|
||||
question_autoclose_enabled=enabled,
|
||||
question_autoclose_hours=hours,
|
||||
question_autoclose_scan_seconds=scan,
|
||||
)
|
||||
|
||||
|
||||
class _FakeGitHub:
|
||||
"""Minimal GitHubBackend stand-in for the scheduler.
|
||||
|
||||
Only `list_comment_reactions` and `close_issue` are exercised; everything
|
||||
else raises so a misuse here surfaces loudly instead of silently.
|
||||
"""
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
*,
|
||||
reactions: Iterable[ReactionInfo] = (),
|
||||
close_error: GitHubError | None = None,
|
||||
) -> None:
|
||||
self._reactions = tuple(reactions)
|
||||
self._close_error = close_error
|
||||
self.close_calls: list[tuple[str, int, str]] = []
|
||||
self.reaction_calls: list[tuple[str, int]] = []
|
||||
|
||||
async def list_comment_reactions(self, repo: str, comment_id: int) -> tuple[ReactionInfo, ...]:
|
||||
self.reaction_calls.append((repo, comment_id))
|
||||
return self._reactions
|
||||
|
||||
async def close_issue(self, repo: str, number: int, *, reason: str = "completed") -> None:
|
||||
self.close_calls.append((repo, number, reason))
|
||||
if self._close_error is not None:
|
||||
raise self._close_error
|
||||
|
||||
|
||||
_KEY = issue_key("octo/widget", 42)
|
||||
|
||||
|
||||
def _seed(db: Database, *, close_at: str = "2000-01-01T00:00:00.000000Z") -> None:
|
||||
db.upsert_pending_closure(
|
||||
issue_key=_KEY,
|
||||
repo="octo/widget",
|
||||
number=42,
|
||||
comment_id=999,
|
||||
issue_author="alice",
|
||||
close_at=close_at,
|
||||
)
|
||||
|
||||
|
||||
async def test_tick_closes_when_no_author_downvote(db: Database) -> None:
|
||||
_seed(db)
|
||||
gh = _FakeGitHub()
|
||||
sched = AutocloseScheduler(settings=_settings(), db=db, github=gh)
|
||||
counts = await sched.tick()
|
||||
assert counts == {"closed": 1, "cancelled": 0, "retried": 0}
|
||||
assert gh.close_calls == [("octo/widget", 42, "completed")]
|
||||
row = db.get_pending_closure(_KEY)
|
||||
assert row is not None and row.state == "closed"
|
||||
assert row.cancel_reason is None
|
||||
|
||||
|
||||
async def test_tick_cancels_when_author_downvotes(db: Database) -> None:
|
||||
_seed(db)
|
||||
gh = _FakeGitHub(
|
||||
reactions=[ReactionInfo(content="-1", user_login="Alice", user_type="User")],
|
||||
)
|
||||
sched = AutocloseScheduler(settings=_settings(), db=db, github=gh)
|
||||
counts = await sched.tick()
|
||||
assert counts == {"closed": 0, "cancelled": 1, "retried": 0}
|
||||
assert gh.close_calls == []
|
||||
row = db.get_pending_closure(_KEY)
|
||||
assert row is not None and row.state == "cancelled"
|
||||
assert row.cancel_reason == "author_downvoted"
|
||||
|
||||
|
||||
async def test_tick_ignores_downvote_from_non_author(db: Database) -> None:
|
||||
"""Watchers / drive-by 👎 from anyone other than the author do not veto."""
|
||||
_seed(db)
|
||||
gh = _FakeGitHub(
|
||||
reactions=[
|
||||
ReactionInfo(content="-1", user_login="rando", user_type="User"),
|
||||
ReactionInfo(content="-1", user_login="some-bot", user_type="Bot"),
|
||||
],
|
||||
)
|
||||
sched = AutocloseScheduler(settings=_settings(), db=db, github=gh)
|
||||
counts = await sched.tick()
|
||||
assert counts == {"closed": 1, "cancelled": 0, "retried": 0}
|
||||
assert gh.close_calls == [("octo/widget", 42, "completed")]
|
||||
|
||||
|
||||
async def test_tick_retries_after_transient_close_error(db: Database) -> None:
|
||||
_seed(db)
|
||||
gh = _FakeGitHub(close_error=GitHubError(502, "Bad Gateway"))
|
||||
sched = AutocloseScheduler(settings=_settings(), db=db, github=gh)
|
||||
counts = await sched.tick()
|
||||
assert counts == {"closed": 0, "cancelled": 0, "retried": 1}
|
||||
row = db.get_pending_closure(_KEY)
|
||||
# Failed attempt resets the row to `pending` so the next tick claims it again.
|
||||
assert row is not None and row.state == "pending"
|
||||
|
||||
|
||||
async def test_tick_treats_404_close_as_already_closed(db: Database) -> None:
|
||||
_seed(db)
|
||||
gh = _FakeGitHub(close_error=GitHubError(404, "Not Found"))
|
||||
sched = AutocloseScheduler(settings=_settings(), db=db, github=gh)
|
||||
counts = await sched.tick()
|
||||
assert counts == {"closed": 0, "cancelled": 1, "retried": 0}
|
||||
row = db.get_pending_closure(_KEY)
|
||||
assert row is not None and row.state == "cancelled"
|
||||
assert row.cancel_reason == "already_closed"
|
||||
|
||||
|
||||
async def test_tick_retries_when_list_reactions_fails(db: Database) -> None:
|
||||
_seed(db)
|
||||
|
||||
class _ReactBoom(_FakeGitHub):
|
||||
async def list_comment_reactions(self, repo, comment_id):
|
||||
raise GitHubError(503, "Service Unavailable")
|
||||
|
||||
gh = _ReactBoom()
|
||||
sched = AutocloseScheduler(settings=_settings(), db=db, github=gh)
|
||||
counts = await sched.tick()
|
||||
assert counts == {"closed": 0, "cancelled": 0, "retried": 1}
|
||||
row = db.get_pending_closure(_KEY)
|
||||
assert row is not None and row.state == "pending"
|
||||
|
||||
|
||||
async def test_tick_skips_future_rows(db: Database) -> None:
|
||||
"""A row whose `close_at` is in the future stays pending."""
|
||||
_seed(db, close_at="2999-01-01T00:00:00.000000Z")
|
||||
gh = _FakeGitHub()
|
||||
sched = AutocloseScheduler(settings=_settings(), db=db, github=gh)
|
||||
counts = await sched.tick()
|
||||
assert counts == {"closed": 0, "cancelled": 0, "retried": 0}
|
||||
assert gh.close_calls == []
|
||||
row = db.get_pending_closure(_KEY)
|
||||
assert row is not None and row.state == "pending"
|
||||
|
||||
|
||||
def test_scheduler_disabled_when_feature_off() -> None:
|
||||
sched = AutocloseScheduler(
|
||||
settings=_settings(enabled=False),
|
||||
db=None, # type: ignore[arg-type]
|
||||
github=None, # type: ignore[arg-type]
|
||||
)
|
||||
assert not sched.enabled
|
||||
|
||||
|
||||
def test_scheduler_disabled_when_hours_zero() -> None:
|
||||
sched = AutocloseScheduler(
|
||||
settings=_settings(hours=0.0),
|
||||
db=None, # type: ignore[arg-type]
|
||||
github=None, # type: ignore[arg-type]
|
||||
)
|
||||
assert not sched.enabled
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_start_is_noop_when_disabled(db: Database) -> None:
|
||||
sched = AutocloseScheduler(
|
||||
settings=_settings(enabled=False),
|
||||
db=db,
|
||||
github=_FakeGitHub(),
|
||||
)
|
||||
await sched.start()
|
||||
# No background task should have been created.
|
||||
assert sched._task is None # type: ignore[attr-defined]
|
||||
await sched.stop() # idempotent
|
||||
@@ -0,0 +1,134 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
from pydantic import ValidationError
|
||||
|
||||
from robomp.config import Settings, reset_settings_cache
|
||||
|
||||
|
||||
def test_settings_load_from_env(env: dict[str, str]) -> None:
|
||||
cfg = Settings() # type: ignore[call-arg]
|
||||
assert cfg.bot_login == "robomp-bot"
|
||||
assert cfg.repo_allowlist == frozenset({"octo/widget"})
|
||||
assert cfg.allows("octo/widget")
|
||||
assert cfg.allows("Octo/Widget")
|
||||
assert not cfg.allows("other/widget")
|
||||
|
||||
|
||||
def test_settings_missing_required(monkeypatch: pytest.MonkeyPatch, env: dict[str, str]) -> None:
|
||||
"""Empty out every credential source: validator MUST trip the
|
||||
'no GitHub access configured' branch. The `env` fixture keeps the other
|
||||
required fields satisfied so we isolate the credential-validator path."""
|
||||
monkeypatch.setenv("GITHUB_TOKEN", "")
|
||||
monkeypatch.setenv("ROBOMP_GH_PROXY_URL", "")
|
||||
monkeypatch.setenv("ROBOMP_GH_PROXY_HMAC_KEY", "")
|
||||
reset_settings_cache()
|
||||
with pytest.raises(ValidationError, match="no GitHub access configured"):
|
||||
Settings() # type: ignore[call-arg]
|
||||
|
||||
|
||||
def test_orchestrator_mode_loads_proxy_config(env: dict[str, str]) -> None:
|
||||
cfg = Settings() # type: ignore[call-arg]
|
||||
assert cfg.github_token is None
|
||||
assert cfg.gh_proxy_url == "http://gh-proxy.invalid:8081"
|
||||
assert cfg.gh_proxy_hmac_key is not None
|
||||
assert cfg.gh_proxy_hmac_key.get_secret_value().startswith("test-hmac-key")
|
||||
|
||||
|
||||
def test_rejects_token_and_proxy_together(monkeypatch: pytest.MonkeyPatch, env: dict[str, str]) -> None:
|
||||
monkeypatch.setenv("GITHUB_TOKEN", "x")
|
||||
reset_settings_cache()
|
||||
with pytest.raises(ValidationError):
|
||||
Settings() # type: ignore[call-arg]
|
||||
|
||||
|
||||
def test_rejects_proxy_url_without_key(monkeypatch: pytest.MonkeyPatch, env: dict[str, str]) -> None:
|
||||
monkeypatch.setenv("ROBOMP_GH_PROXY_HMAC_KEY", "")
|
||||
reset_settings_cache()
|
||||
with pytest.raises(ValidationError):
|
||||
Settings() # type: ignore[call-arg]
|
||||
|
||||
|
||||
def test_proxy_mode_loads_pat(proxy_env: dict[str, str]) -> None:
|
||||
cfg = Settings() # type: ignore[call-arg]
|
||||
assert cfg.github_token is not None
|
||||
assert cfg.github_token.get_secret_value() == "ghp_test_token_value_xxxxxxxxxxxxxxxx"
|
||||
assert cfg.gh_proxy_url is None
|
||||
assert cfg.gh_proxy_hmac_key is None
|
||||
|
||||
|
||||
def test_allowlist_csv_parsing(monkeypatch: pytest.MonkeyPatch, env: dict[str, str]) -> None:
|
||||
monkeypatch.setenv("ROBOMP_REPO_ALLOWLIST", " alpha/one ,beta/two, ,gamma/three ")
|
||||
reset_settings_cache()
|
||||
cfg = Settings() # type: ignore[call-arg]
|
||||
assert cfg.repo_allowlist == frozenset({"alpha/one", "beta/two", "gamma/three"})
|
||||
|
||||
|
||||
def test_blank_replay_token_treated_as_disabled(monkeypatch: pytest.MonkeyPatch, env: dict[str, str]) -> None:
|
||||
monkeypatch.setenv("ROBOMP_REPLAY_TOKEN", "")
|
||||
reset_settings_cache()
|
||||
cfg = Settings() # type: ignore[call-arg]
|
||||
assert cfg.replay_token is None
|
||||
|
||||
|
||||
def test_whitespace_replay_token_treated_as_disabled(monkeypatch: pytest.MonkeyPatch, env: dict[str, str]) -> None:
|
||||
monkeypatch.setenv("ROBOMP_REPLAY_TOKEN", " ")
|
||||
reset_settings_cache()
|
||||
cfg = Settings() # type: ignore[call-arg]
|
||||
assert cfg.replay_token is None
|
||||
|
||||
|
||||
def test_real_replay_token_preserved(monkeypatch: pytest.MonkeyPatch, env: dict[str, str]) -> None:
|
||||
monkeypatch.setenv("ROBOMP_REPLAY_TOKEN", "abc")
|
||||
reset_settings_cache()
|
||||
cfg = Settings() # type: ignore[call-arg]
|
||||
assert cfg.replay_token is not None
|
||||
assert cfg.replay_token.get_secret_value() == "abc"
|
||||
|
||||
|
||||
def test_blank_bot_login_rejected(monkeypatch: pytest.MonkeyPatch, env: dict[str, str]) -> None:
|
||||
monkeypatch.setenv("ROBOMP_BOT_LOGIN", " ")
|
||||
reset_settings_cache()
|
||||
with pytest.raises(ValidationError):
|
||||
Settings() # type: ignore[call-arg]
|
||||
|
||||
|
||||
def test_model_pool_single(env: dict[str, str]) -> None:
|
||||
cfg = Settings() # type: ignore[call-arg]
|
||||
assert cfg.model_pool == (cfg.model,)
|
||||
assert cfg.pick_model() == cfg.model
|
||||
|
||||
|
||||
def test_model_pool_csv_parses(monkeypatch: pytest.MonkeyPatch, env: dict[str, str]) -> None:
|
||||
monkeypatch.setenv(
|
||||
"ROBOMP_MODEL",
|
||||
" p-codex/gpt-5.4 , p-anthropic/claude-sonnet-4-6 ,, p-anthropic/claude-opus-4-7 ",
|
||||
)
|
||||
reset_settings_cache()
|
||||
cfg = Settings() # type: ignore[call-arg]
|
||||
assert cfg.model_pool == (
|
||||
"p-codex/gpt-5.4",
|
||||
"p-anthropic/claude-sonnet-4-6",
|
||||
"p-anthropic/claude-opus-4-7",
|
||||
)
|
||||
|
||||
|
||||
def test_pick_model_covers_full_pool(monkeypatch: pytest.MonkeyPatch, env: dict[str, str]) -> None:
|
||||
"""With a 3-item pool and 500 picks, each option appears at least once."""
|
||||
monkeypatch.setenv("ROBOMP_MODEL", "a,b,c")
|
||||
reset_settings_cache()
|
||||
cfg = Settings() # type: ignore[call-arg]
|
||||
seen = {cfg.pick_model() for _ in range(500)}
|
||||
assert seen == {"a", "b", "c"}
|
||||
|
||||
|
||||
def test_max_concurrency_default_is_8(env: dict[str, str]) -> None:
|
||||
cfg = Settings() # type: ignore[call-arg]
|
||||
assert cfg.max_concurrency == 8
|
||||
|
||||
|
||||
def test_task_timeout_hard_grace_env_parses(monkeypatch: pytest.MonkeyPatch, env: dict[str, str]) -> None:
|
||||
monkeypatch.setenv("ROBOMP_TASK_TIMEOUT_HARD_GRACE_SECONDS", "12.5")
|
||||
reset_settings_cache()
|
||||
cfg = Settings() # type: ignore[call-arg]
|
||||
assert cfg.task_timeout_hard_grace_seconds == 12.5
|
||||
@@ -0,0 +1,562 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import threading
|
||||
from concurrent.futures import ThreadPoolExecutor
|
||||
from pathlib import Path
|
||||
|
||||
from robomp.db import Database, iso_seconds_ago, issue_key
|
||||
|
||||
|
||||
def test_record_event_dedupes_by_delivery(db: Database) -> None:
|
||||
payload = {"action": "opened", "issue": {"number": 1}}
|
||||
assert db.record_event(
|
||||
delivery_id="abc",
|
||||
event_type="issues",
|
||||
repo="octo/widget",
|
||||
issue_key=issue_key("octo/widget", 1),
|
||||
payload=payload,
|
||||
)
|
||||
assert not db.record_event(
|
||||
delivery_id="abc",
|
||||
event_type="issues",
|
||||
repo="octo/widget",
|
||||
issue_key=issue_key("octo/widget", 1),
|
||||
payload=payload,
|
||||
)
|
||||
|
||||
|
||||
def test_claim_next_event_singleton_under_contention(db: Database) -> None:
|
||||
for i in range(5):
|
||||
db.record_event(
|
||||
delivery_id=f"d-{i}",
|
||||
event_type="issues",
|
||||
repo="octo/widget",
|
||||
issue_key=issue_key("octo/widget", i),
|
||||
payload={"i": i},
|
||||
)
|
||||
|
||||
winners: list[str] = []
|
||||
lock = threading.Lock()
|
||||
|
||||
def claim() -> None:
|
||||
row = db.claim_next_event()
|
||||
if row is not None:
|
||||
with lock:
|
||||
winners.append(row.delivery_id)
|
||||
|
||||
with ThreadPoolExecutor(max_workers=8) as pool:
|
||||
for _ in range(5):
|
||||
futures = [pool.submit(claim) for _ in range(8)]
|
||||
for f in futures:
|
||||
f.result()
|
||||
|
||||
# Each delivery id should appear exactly once.
|
||||
assert sorted(winners) == [f"d-{i}" for i in range(5)]
|
||||
assert all(db.get_event(f"d-{i}").state == "running" for i in range(5))
|
||||
|
||||
|
||||
def test_requeue_event_can_be_restricted_by_source_state(db: Database) -> None:
|
||||
db.record_event(
|
||||
delivery_id="done-event",
|
||||
event_type="issues",
|
||||
repo="octo/widget",
|
||||
issue_key=issue_key("octo/widget", 1),
|
||||
payload={},
|
||||
state="done",
|
||||
)
|
||||
db.record_event(
|
||||
delivery_id="running-event",
|
||||
event_type="issues",
|
||||
repo="octo/widget",
|
||||
issue_key=issue_key("octo/widget", 2),
|
||||
payload={},
|
||||
state="running",
|
||||
)
|
||||
|
||||
assert db.requeue_event("done-event", from_states=("done", "failed", "skipped"))
|
||||
assert db.get_event("done-event").state == "queued"
|
||||
|
||||
assert not db.requeue_event("running-event", from_states=("done", "failed", "skipped"))
|
||||
assert db.get_event("running-event").state == "running"
|
||||
|
||||
|
||||
def test_latest_issue_events_ignore_skipped_noise(db: Database) -> None:
|
||||
fixed = issue_key("octo/widget", 1)
|
||||
still_failed = issue_key("octo/widget", 2)
|
||||
db.record_event(
|
||||
delivery_id="fixed-failed",
|
||||
event_type="issues",
|
||||
repo="octo/widget",
|
||||
issue_key=fixed,
|
||||
payload={"action": "opened"},
|
||||
state="failed",
|
||||
)
|
||||
db.record_event(
|
||||
delivery_id="fixed-done",
|
||||
event_type="issues",
|
||||
repo="octo/widget",
|
||||
issue_key=fixed,
|
||||
payload={"action": "opened"},
|
||||
state="done",
|
||||
)
|
||||
db.record_event(
|
||||
delivery_id="failed-run",
|
||||
event_type="issues",
|
||||
repo="octo/widget",
|
||||
issue_key=still_failed,
|
||||
payload={"action": "opened"},
|
||||
state="failed",
|
||||
)
|
||||
db.record_event(
|
||||
delivery_id="label-noise",
|
||||
event_type="issues",
|
||||
repo="octo/widget",
|
||||
issue_key=still_failed,
|
||||
payload={"action": "labeled"},
|
||||
state="skipped",
|
||||
last_error="issues.labeled ignored",
|
||||
)
|
||||
|
||||
latest_failed = db.latest_event_for_issue(still_failed)
|
||||
latest_raw = db.latest_event_for_issue(still_failed, include_skipped=True)
|
||||
assert latest_failed is not None
|
||||
assert latest_raw is not None
|
||||
assert latest_failed.delivery_id == "failed-run"
|
||||
assert latest_raw.delivery_id == "label-noise"
|
||||
|
||||
latest = db.latest_events_for_issues((fixed, still_failed))
|
||||
assert latest[fixed].delivery_id == "fixed-done"
|
||||
assert latest[still_failed].delivery_id == "failed-run"
|
||||
|
||||
counts = db.latest_issue_event_state_counts()
|
||||
assert counts["done"] == 1
|
||||
assert counts["failed"] == 1
|
||||
assert counts["skipped"] == 0
|
||||
|
||||
|
||||
def test_reset_stuck_running_recovers(db: Database) -> None:
|
||||
db.record_event(
|
||||
delivery_id="d1",
|
||||
event_type="issues",
|
||||
repo="octo/widget",
|
||||
issue_key="octo/widget#1",
|
||||
payload={},
|
||||
)
|
||||
row = db.claim_next_event()
|
||||
assert row is not None
|
||||
# Capture `started_at` set by the claim so we can prove the recovery flip preserves it.
|
||||
with db._lock: # noqa: SLF001
|
||||
before = db._conn.execute( # noqa: SLF001
|
||||
"SELECT started_at FROM events WHERE delivery_id=?", ("d1",)
|
||||
).fetchone()
|
||||
assert before is not None
|
||||
assert before["started_at"] is not None
|
||||
# Simulate crash: row still running.
|
||||
recovered = db.reset_stuck_running()
|
||||
assert recovered == 1
|
||||
assert db.get_event("d1").state == "queued"
|
||||
with db._lock: # noqa: SLF001
|
||||
after = db._conn.execute( # noqa: SLF001
|
||||
"SELECT started_at FROM events WHERE delivery_id=?", ("d1",)
|
||||
).fetchone()
|
||||
assert after is not None
|
||||
assert after["started_at"] == before["started_at"]
|
||||
|
||||
|
||||
def test_upsert_issue_round_trip(db: Database) -> None:
|
||||
key = issue_key("octo/widget", 7)
|
||||
row = db.upsert_issue(
|
||||
key=key,
|
||||
repo="octo/widget",
|
||||
number=7,
|
||||
state="new",
|
||||
)
|
||||
assert row.state == "new"
|
||||
row = db.upsert_issue(
|
||||
key=key,
|
||||
repo="octo/widget",
|
||||
number=7,
|
||||
state="opened",
|
||||
branch="farm/abcd1234/some-issue",
|
||||
session_dir="/tmp/s",
|
||||
pr_number=42,
|
||||
)
|
||||
assert row.state == "opened"
|
||||
assert row.branch == "farm/abcd1234/some-issue"
|
||||
assert row.pr_number == 42
|
||||
fetched = db.get_issue(key)
|
||||
assert fetched and fetched.pr_number == 42
|
||||
|
||||
found = db.find_issue_by_pr("octo/widget", 42)
|
||||
assert found and found.key == key
|
||||
by_branch = db.find_issue_by_branch("octo/widget", "farm/abcd1234/some-issue")
|
||||
assert by_branch and by_branch.key == key
|
||||
|
||||
|
||||
def test_log_tool_call(db: Database) -> None:
|
||||
db.upsert_issue(key="octo/widget#1", repo="octo/widget", number=1, state="new")
|
||||
row_id = db.log_tool_call(
|
||||
issue_key="octo/widget#1",
|
||||
tool="gh_post_comment",
|
||||
args={"body": "hi"},
|
||||
result={"comment_id": 9},
|
||||
)
|
||||
assert row_id > 0
|
||||
|
||||
|
||||
def test_processed_issue_keys_returns_only_known(db: Database) -> None:
|
||||
db.upsert_issue(key=issue_key("octo/widget", 1), repo="octo/widget", number=1, state="new")
|
||||
db.upsert_issue(key=issue_key("octo/widget", 2), repo="octo/widget", number=2, state="reproducing")
|
||||
queried = [
|
||||
issue_key("octo/widget", 1),
|
||||
issue_key("octo/widget", 2),
|
||||
issue_key("octo/widget", 3), # never upserted
|
||||
issue_key("octo/other", 7), # different repo, never upserted
|
||||
]
|
||||
result = db.processed_issue_keys(queried)
|
||||
assert result == {issue_key("octo/widget", 1), issue_key("octo/widget", 2)}
|
||||
|
||||
|
||||
def test_processed_issue_keys_empty_input(db: Database) -> None:
|
||||
assert db.processed_issue_keys([]) == set()
|
||||
# Empty strings are filtered out, not sent as a parameter.
|
||||
assert db.processed_issue_keys(["", ""]) == set()
|
||||
|
||||
|
||||
def test_processed_issue_keys_handles_large_batch(db: Database) -> None:
|
||||
# Confirms the 500-batch chunking path (>500 parameters would otherwise hit
|
||||
# SQLite's SQLITE_MAX_VARIABLE_NUMBER default of 999 on older builds).
|
||||
keys = [issue_key("octo/widget", n) for n in range(1, 750)]
|
||||
# Persist only every 3rd one.
|
||||
for k, n in zip(keys, range(1, 750), strict=True):
|
||||
if n % 3 == 0:
|
||||
db.upsert_issue(key=k, repo="octo/widget", number=n, state="new")
|
||||
result = db.processed_issue_keys(keys + ["bogus#1"])
|
||||
expected = {issue_key("octo/widget", n) for n in range(1, 750) if n % 3 == 0}
|
||||
assert result == expected
|
||||
|
||||
|
||||
def test_classification_roundtrip(db: Database) -> None:
|
||||
key = issue_key("octo/widget", 7)
|
||||
db.upsert_issue(key=key, repo="octo/widget", number=7, state="new")
|
||||
row = db.get_issue(key)
|
||||
assert row is not None and row.classification is None
|
||||
db.set_issue_classification(key, "question")
|
||||
row = db.get_issue(key)
|
||||
assert row is not None and row.classification == "question"
|
||||
# Round-trip via list_issues too.
|
||||
items = db.list_issues()
|
||||
assert any(r.key == key and r.classification == "question" for r in items)
|
||||
|
||||
|
||||
def test_migration_adds_classification_to_existing_db(tmp_path: Path) -> None:
|
||||
"""Open a DB without the classification column and verify the migration."""
|
||||
import sqlite3
|
||||
|
||||
path = tmp_path / "legacy.sqlite"
|
||||
conn = sqlite3.connect(str(path))
|
||||
conn.executescript(
|
||||
"""
|
||||
CREATE TABLE events (delivery_id TEXT PRIMARY KEY, event_type TEXT, payload_json TEXT,
|
||||
received_at TEXT, state TEXT CHECK(state IN ('queued','running','done','failed','skipped')),
|
||||
attempts INTEGER DEFAULT 0, last_error TEXT, repo TEXT, issue_key TEXT,
|
||||
started_at TEXT, finished_at TEXT);
|
||||
CREATE TABLE issues (key TEXT PRIMARY KEY, repo TEXT, number INTEGER, branch TEXT,
|
||||
session_dir TEXT, pr_number INTEGER, state TEXT, updated_at TEXT);
|
||||
CREATE TABLE tool_calls (id INTEGER PRIMARY KEY AUTOINCREMENT, issue_key TEXT,
|
||||
tool TEXT, args_json TEXT, result_json TEXT, error TEXT, ts TEXT);
|
||||
INSERT INTO issues VALUES ('octo/widget#1', 'octo/widget', 1, 'farm/x', '/tmp/s', NULL,
|
||||
'reproducing', '2026-01-01T00:00:00Z');
|
||||
"""
|
||||
)
|
||||
conn.commit()
|
||||
conn.close()
|
||||
# Opening through our Database class should auto-migrate.
|
||||
database = Database(path)
|
||||
row = database.get_issue("octo/widget#1")
|
||||
assert row is not None
|
||||
assert row.classification is None # column exists, default NULL
|
||||
database.set_issue_classification("octo/widget#1", "bug")
|
||||
assert database.get_issue("octo/widget#1").classification == "bug"
|
||||
database.close()
|
||||
|
||||
|
||||
def test_set_event_model_persists_on_running_event(db: Database) -> None:
|
||||
"""`set_event_model` writes the picked model so the dashboard can attribute behavior."""
|
||||
db.record_event(
|
||||
delivery_id="d-model",
|
||||
event_type="issues",
|
||||
repo="octo/widget",
|
||||
issue_key=issue_key("octo/widget", 42),
|
||||
payload={"action": "opened"},
|
||||
)
|
||||
row = db.claim_next_event()
|
||||
assert row is not None and row.delivery_id == "d-model"
|
||||
db.set_event_model("d-model", "claude-sonnet-4-5")
|
||||
running = db.list_running_events()
|
||||
assert len(running) == 1
|
||||
assert running[0]["model"] == "claude-sonnet-4-5"
|
||||
# Setting a different model later (e.g. retry) overwrites in place.
|
||||
db.set_event_model("d-model", "claude-opus-4-5")
|
||||
running = db.list_running_events()
|
||||
assert running[0]["model"] == "claude-opus-4-5"
|
||||
|
||||
|
||||
def test_list_running_events_surfaces_last_tool_since_start(db: Database) -> None:
|
||||
"""`list_running_events` joins the most recent tool_call newer than `started_at`.
|
||||
|
||||
Tool calls logged before the current run (e.g. an earlier triage on the same
|
||||
issue) MUST NOT be reported as the current activity.
|
||||
"""
|
||||
key = issue_key("octo/widget", 7)
|
||||
db.upsert_issue(key=key, repo="octo/widget", number=7, state="reproducing")
|
||||
# Stale tool call from a previous run (no started_at yet).
|
||||
db.log_tool_call(issue_key=key, tool="stale_tool", args={})
|
||||
db.record_event(
|
||||
delivery_id="d-7",
|
||||
event_type="issues",
|
||||
repo="octo/widget",
|
||||
issue_key=key,
|
||||
payload={"action": "opened"},
|
||||
)
|
||||
db.claim_next_event() # sets started_at
|
||||
# Before any current-run tool call: last_tool must be NULL, not "stale_tool".
|
||||
running = db.list_running_events()
|
||||
assert len(running) == 1
|
||||
assert running[0]["last_tool"] is None
|
||||
assert running[0]["last_tool_ts"] is None
|
||||
# New tool call after start → surfaces in the snapshot.
|
||||
db.log_tool_call(issue_key=key, tool="gh_post_comment", args={"body": "hi"})
|
||||
db.log_tool_call(issue_key=key, tool="set_issue_labels", args={"labels": ["bug"]})
|
||||
running = db.list_running_events()
|
||||
assert running[0]["last_tool"] == "set_issue_labels" # latest by ts
|
||||
assert running[0]["last_tool_ts"] is not None
|
||||
|
||||
|
||||
def test_record_submission_dedupes_by_delivery(db: Database) -> None:
|
||||
assert db.record_submission(delivery_id="d-1", login="Alice", repo="octo/widget")
|
||||
# Retry of the same delivery id is a no-op (idempotent webhook delivery).
|
||||
assert not db.record_submission(delivery_id="d-1", login="alice", repo="octo/widget")
|
||||
|
||||
|
||||
def test_admit_submission_dedupes_by_delivery_before_rate_limit(db: Database) -> None:
|
||||
since = iso_seconds_ago(60)
|
||||
first = db.admit_submission(
|
||||
delivery_id="d-1",
|
||||
login="Alice",
|
||||
repo="octo/widget",
|
||||
since=since,
|
||||
cap=1,
|
||||
)
|
||||
assert first.accepted
|
||||
assert not first.duplicate
|
||||
assert first.used == 1
|
||||
|
||||
duplicate = db.admit_submission(
|
||||
delivery_id="d-1",
|
||||
login="alice",
|
||||
repo="octo/widget",
|
||||
since=since,
|
||||
cap=1,
|
||||
)
|
||||
assert duplicate.accepted
|
||||
assert duplicate.duplicate
|
||||
assert duplicate.used == 1
|
||||
|
||||
rejected = db.admit_submission(
|
||||
delivery_id="d-2",
|
||||
login="ALICE",
|
||||
repo="octo/widget",
|
||||
since=since,
|
||||
cap=1,
|
||||
)
|
||||
assert not rejected.accepted
|
||||
assert not rejected.duplicate
|
||||
assert rejected.used == 1
|
||||
assert db.count_submissions_since("alice", since) == 1
|
||||
|
||||
|
||||
def test_admit_submission_enforces_cap_atomically_across_connections(tmp_path: Path) -> None:
|
||||
path = tmp_path / "admission.sqlite"
|
||||
# Pre-warm: open + migrate the schema once so the two racing threads below
|
||||
# collide only on `admit_submission` (which is what the test is exercising),
|
||||
# not on `Database.__init__`. `executescript(SCHEMA)` flips journal_mode to
|
||||
# WAL, which needs a brief exclusive lock — without pre-warming, one
|
||||
# thread can lose that race and never reach `barrier.wait()`, deadlocking
|
||||
# its peer at the barrier (no timeout) and hanging `future.result()`.
|
||||
Database(path).close()
|
||||
barrier = threading.Barrier(2, timeout=10)
|
||||
|
||||
def admit(delivery_id: str) -> bool:
|
||||
database = Database(path)
|
||||
try:
|
||||
barrier.wait()
|
||||
return database.admit_submission(
|
||||
delivery_id=delivery_id,
|
||||
login="alice",
|
||||
repo="octo/widget",
|
||||
since=iso_seconds_ago(60),
|
||||
cap=1,
|
||||
).accepted
|
||||
finally:
|
||||
database.close()
|
||||
|
||||
with ThreadPoolExecutor(max_workers=2) as pool:
|
||||
futures = [pool.submit(admit, f"d-{i}") for i in range(2)]
|
||||
accepted = [future.result(timeout=15) for future in futures]
|
||||
|
||||
verifier = Database(path)
|
||||
try:
|
||||
assert sorted(accepted) == [False, True]
|
||||
assert verifier.count_submissions_since("alice", iso_seconds_ago(60)) == 1
|
||||
finally:
|
||||
verifier.close()
|
||||
|
||||
|
||||
def test_count_submissions_since_is_case_insensitive(db: Database) -> None:
|
||||
db.record_submission(delivery_id="d-1", login="Alice", repo="octo/widget")
|
||||
db.record_submission(delivery_id="d-2", login="ALICE", repo="octo/widget")
|
||||
db.record_submission(delivery_id="d-3", login="bob", repo="octo/widget")
|
||||
# Window covering the whole test run.
|
||||
since = iso_seconds_ago(60)
|
||||
assert db.count_submissions_since("alice", since) == 2
|
||||
assert db.count_submissions_since("ALICE", since) == 2
|
||||
assert db.count_submissions_since("bob", since) == 1
|
||||
assert db.count_submissions_since("nobody", since) == 0
|
||||
|
||||
|
||||
def test_count_submissions_since_respects_window(db: Database) -> None:
|
||||
db.record_submission(delivery_id="d-1", login="alice", repo="octo/widget")
|
||||
# Future cutoff means the just-inserted row is *before* the window.
|
||||
future = iso_seconds_ago(-60)
|
||||
assert db.count_submissions_since("alice", future) == 0
|
||||
|
||||
|
||||
# -------- pending_closures ---------------------------------------------
|
||||
|
||||
|
||||
_KEY = issue_key("octo/widget", 42)
|
||||
|
||||
|
||||
def _seed_pending(db: Database, *, close_at: str = "2026-05-15T00:00:00.000000Z") -> None:
|
||||
db.upsert_pending_closure(
|
||||
issue_key=_KEY,
|
||||
repo="octo/widget",
|
||||
number=42,
|
||||
comment_id=999,
|
||||
issue_author="Alice",
|
||||
close_at=close_at,
|
||||
)
|
||||
|
||||
|
||||
def test_upsert_pending_closure_lowercases_author_and_starts_pending(db: Database) -> None:
|
||||
_seed_pending(db)
|
||||
row = db.get_pending_closure(_KEY)
|
||||
assert row is not None
|
||||
assert row.state == "pending"
|
||||
assert row.cancel_reason is None
|
||||
assert row.issue_author == "alice" # author stored lower-cased for cheap eq
|
||||
assert row.comment_id == 999
|
||||
|
||||
|
||||
def test_upsert_pending_closure_overwrites_prior_schedule(db: Database) -> None:
|
||||
_seed_pending(db)
|
||||
db.finalize_closure(_KEY, state="cancelled", reason="user_replied")
|
||||
# A follow-up bot answer should reset the row to pending and update fields.
|
||||
db.upsert_pending_closure(
|
||||
issue_key=_KEY,
|
||||
repo="octo/widget",
|
||||
number=42,
|
||||
comment_id=1234,
|
||||
issue_author="alice",
|
||||
close_at="2030-01-01T00:00:00.000000Z",
|
||||
)
|
||||
row = db.get_pending_closure(_KEY)
|
||||
assert row is not None
|
||||
assert row.state == "pending"
|
||||
assert row.cancel_reason is None
|
||||
assert row.comment_id == 1234
|
||||
assert row.close_at == "2030-01-01T00:00:00.000000Z"
|
||||
|
||||
|
||||
def test_claim_due_closures_only_returns_due_pending(db: Database) -> None:
|
||||
_seed_pending(db, close_at="2000-01-01T00:00:00.000000Z") # past
|
||||
db.upsert_pending_closure(
|
||||
issue_key=issue_key("octo/widget", 7),
|
||||
repo="octo/widget",
|
||||
number=7,
|
||||
comment_id=10,
|
||||
issue_author="bob",
|
||||
close_at="2999-01-01T00:00:00.000000Z", # future
|
||||
)
|
||||
claimed = db.claim_due_closures(now="2026-05-15T00:00:00.000000Z")
|
||||
assert [r.issue_key for r in claimed] == [_KEY]
|
||||
assert all(r.state == "claimed" for r in claimed)
|
||||
# And re-claiming returns nothing because the first one is no longer pending.
|
||||
again = db.claim_due_closures(now="2026-05-15T00:00:00.000000Z")
|
||||
assert again == []
|
||||
|
||||
|
||||
def test_claim_due_closures_atomic_under_contention(db: Database) -> None:
|
||||
"""Two concurrent claims see disjoint rows."""
|
||||
for n in range(5):
|
||||
db.upsert_pending_closure(
|
||||
issue_key=issue_key("octo/widget", n),
|
||||
repo="octo/widget",
|
||||
number=n,
|
||||
comment_id=100 + n,
|
||||
issue_author="alice",
|
||||
close_at="2000-01-01T00:00:00.000000Z",
|
||||
)
|
||||
seen: list[str] = []
|
||||
lock = threading.Lock()
|
||||
|
||||
def claim_some() -> None:
|
||||
rows = db.claim_due_closures(now="2026-05-15T00:00:00.000000Z", limit=2)
|
||||
with lock:
|
||||
seen.extend(r.issue_key for r in rows)
|
||||
|
||||
with ThreadPoolExecutor(max_workers=4) as pool:
|
||||
for _ in range(4):
|
||||
list(pool.map(lambda _: claim_some(), range(4)))
|
||||
# Each row must appear at most once across all claims.
|
||||
assert sorted(seen) == sorted({issue_key("octo/widget", n) for n in range(5)})
|
||||
|
||||
|
||||
def test_cancel_pending_closure_only_fires_when_pending(db: Database) -> None:
|
||||
_seed_pending(db)
|
||||
assert db.cancel_pending_closure(_KEY, reason="user_replied")
|
||||
row = db.get_pending_closure(_KEY)
|
||||
assert row is not None
|
||||
assert row.state == "cancelled"
|
||||
assert row.cancel_reason == "user_replied"
|
||||
# A second cancel against an already-cancelled row is a no-op.
|
||||
assert not db.cancel_pending_closure(_KEY, reason="user_replied")
|
||||
|
||||
|
||||
def test_cancel_pending_closure_skips_claimed_rows(db: Database) -> None:
|
||||
"""A `claimed` row must be left for the scheduler tick that owns it."""
|
||||
_seed_pending(db, close_at="2000-01-01T00:00:00.000000Z")
|
||||
claimed = db.claim_due_closures(now="2026-05-15T00:00:00.000000Z")
|
||||
assert claimed and claimed[0].state == "claimed"
|
||||
assert not db.cancel_pending_closure(_KEY, reason="user_replied")
|
||||
row = db.get_pending_closure(_KEY)
|
||||
assert row is not None and row.state == "claimed"
|
||||
|
||||
|
||||
def test_finalize_closure_rejects_non_terminal_state(db: Database) -> None:
|
||||
_seed_pending(db)
|
||||
import pytest
|
||||
|
||||
with pytest.raises(ValueError):
|
||||
db.finalize_closure(_KEY, state="pending", reason=None) # type: ignore[arg-type]
|
||||
|
||||
|
||||
def test_requeue_claimed_closure_only_flips_claimed(db: Database) -> None:
|
||||
_seed_pending(db, close_at="2000-01-01T00:00:00.000000Z")
|
||||
db.claim_due_closures(now="2026-05-15T00:00:00.000000Z")
|
||||
assert db.requeue_claimed_closure(_KEY)
|
||||
row = db.get_pending_closure(_KEY)
|
||||
assert row is not None and row.state == "pending"
|
||||
# Now in pending state, requeue is a no-op.
|
||||
assert not db.requeue_claimed_closure(_KEY)
|
||||
@@ -0,0 +1,222 @@
|
||||
"""GitHub REST client tests against httpx.MockTransport."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
|
||||
from robomp.github_client import GitHubClient, GitHubError
|
||||
|
||||
|
||||
def _run_async(coro):
|
||||
return asyncio.new_event_loop().run_until_complete(coro)
|
||||
|
||||
|
||||
def test_4xx_maps_to_github_error_with_message() -> None:
|
||||
transport = httpx.MockTransport(lambda req: httpx.Response(404, json={"message": "Not Found"}))
|
||||
client = GitHubClient("tok", transport=transport)
|
||||
with pytest.raises(GitHubError) as exc:
|
||||
asyncio.new_event_loop().run_until_complete(client.get_repo("o/r"))
|
||||
assert exc.value.status == 404
|
||||
assert "Not Found" in str(exc.value)
|
||||
|
||||
|
||||
def test_rate_limit_retry_after_parsed() -> None:
|
||||
transport = httpx.MockTransport(
|
||||
lambda req: httpx.Response(
|
||||
403,
|
||||
json={"message": "rate limited"},
|
||||
headers={"retry-after": "42"},
|
||||
)
|
||||
)
|
||||
client = GitHubClient("tok", transport=transport)
|
||||
with pytest.raises(GitHubError) as exc:
|
||||
asyncio.new_event_loop().run_until_complete(client.get_repo("o/r"))
|
||||
assert exc.value.retry_after == 42.0
|
||||
|
||||
|
||||
def test_redirect_without_follow_raises_github_error() -> None:
|
||||
"""If a moved repo returns 301 and the redirect target is unreachable,
|
||||
we must raise a clean GitHubError instead of parsing the response body."""
|
||||
calls: list[str] = []
|
||||
|
||||
def handler(request: httpx.Request) -> httpx.Response:
|
||||
calls.append(str(request.url))
|
||||
# First request: simulate a 301 redirect that the client cannot follow
|
||||
# because the new location resolves to a 410 Gone.
|
||||
if len(calls) == 1:
|
||||
return httpx.Response(
|
||||
301,
|
||||
headers={"location": "https://api.github.com/repositories/12345"},
|
||||
)
|
||||
return httpx.Response(410, json={"message": "Gone"})
|
||||
|
||||
transport = httpx.MockTransport(handler)
|
||||
client = GitHubClient("tok", transport=transport)
|
||||
with pytest.raises(GitHubError) as exc:
|
||||
asyncio.new_event_loop().run_until_complete(client.get_repo("old-owner/old-repo"))
|
||||
# Either we end up at 410 after following, or we surface the redirect itself
|
||||
# — both are GitHubError, not an internal exception.
|
||||
assert exc.value.status in (301, 410)
|
||||
|
||||
|
||||
def test_redirect_target_succeeds_when_followable() -> None:
|
||||
"""A 301 → 200 chain should resolve to the followed payload."""
|
||||
|
||||
def handler(request: httpx.Request) -> httpx.Response:
|
||||
if request.url.path == "/repos/old/repo":
|
||||
return httpx.Response(
|
||||
301,
|
||||
headers={"location": "https://api.github.com/repos/new/repo"},
|
||||
)
|
||||
return httpx.Response(
|
||||
200,
|
||||
json={
|
||||
"full_name": "new/repo",
|
||||
"default_branch": "main",
|
||||
"clone_url": "https://github.com/new/repo.git",
|
||||
"private": False,
|
||||
},
|
||||
)
|
||||
|
||||
transport = httpx.MockTransport(handler)
|
||||
client = GitHubClient("tok", transport=transport)
|
||||
repo = asyncio.new_event_loop().run_until_complete(client.get_repo("old/repo"))
|
||||
assert repo.full_name == "new/repo"
|
||||
|
||||
|
||||
def test_get_pull_request_parses_head_repo_and_author() -> None:
|
||||
def handler(request: httpx.Request) -> httpx.Response:
|
||||
assert request.url.path == "/repos/octo/widget/pulls/9"
|
||||
return httpx.Response(
|
||||
200,
|
||||
json={
|
||||
"number": 9,
|
||||
"html_url": "https://github.com/octo/widget/pull/9",
|
||||
"head": {"ref": "farm/abc12345/fix", "repo": {"full_name": "octo/widget"}},
|
||||
"base": {"ref": "main"},
|
||||
"state": "open",
|
||||
"user": {"login": "robomp-bot"},
|
||||
},
|
||||
)
|
||||
|
||||
client = GitHubClient("tok", transport=httpx.MockTransport(handler))
|
||||
pr = _run_async(client.get_pull_request("octo/widget", 9))
|
||||
assert pr.head_ref == "farm/abc12345/fix"
|
||||
assert pr.head_repo == "octo/widget"
|
||||
assert pr.author == "robomp-bot"
|
||||
|
||||
|
||||
def test_204_no_content_returns_none() -> None:
|
||||
transport = httpx.MockTransport(lambda r: httpx.Response(204))
|
||||
client = GitHubClient("tok", transport=transport)
|
||||
# add_assignees with empty list short-circuits without a request; pass one to force the call.
|
||||
asyncio.new_event_loop().run_until_complete(client.add_assignees("o/r", 1, ["alice"]))
|
||||
|
||||
|
||||
def test_list_closing_pull_requests_filters_disconnected_and_closed() -> None:
|
||||
"""Net connected−disconnected open PRs only."""
|
||||
captured: dict[str, str] = {}
|
||||
|
||||
timeline = [
|
||||
# PR #100 connected and still open → included
|
||||
{
|
||||
"event": "connected",
|
||||
"source": {"issue": {"number": 100, "state": "open", "pull_request": {"url": "..."}}},
|
||||
},
|
||||
# PR #200 connected then disconnected → excluded
|
||||
{
|
||||
"event": "connected",
|
||||
"source": {"issue": {"number": 200, "state": "open", "pull_request": {"url": "..."}}},
|
||||
},
|
||||
{
|
||||
"event": "disconnected",
|
||||
"source": {"issue": {"number": 200, "state": "open", "pull_request": {"url": "..."}}},
|
||||
},
|
||||
# PR #300 connected but currently closed (e.g. rejected) → excluded
|
||||
{
|
||||
"event": "connected",
|
||||
"source": {"issue": {"number": 300, "state": "closed", "pull_request": {"url": "..."}}},
|
||||
},
|
||||
# Cross-referenced (not connected) — not a closing link → excluded
|
||||
{
|
||||
"event": "cross-referenced",
|
||||
"source": {"issue": {"number": 400, "state": "open", "pull_request": {"url": "..."}}},
|
||||
},
|
||||
# Plain issue cross-ref (no pull_request) → excluded
|
||||
{
|
||||
"event": "connected",
|
||||
"source": {"issue": {"number": 500, "state": "open"}},
|
||||
},
|
||||
# Unrelated timeline events → ignored
|
||||
{"event": "labeled", "label": {"name": "bug"}},
|
||||
]
|
||||
|
||||
def handler(request: httpx.Request) -> httpx.Response:
|
||||
captured["path"] = request.url.path
|
||||
captured["per_page"] = request.url.params.get("per_page", "")
|
||||
return httpx.Response(200, json=timeline)
|
||||
|
||||
client = GitHubClient("tok", transport=httpx.MockTransport(handler))
|
||||
prs = _run_async(client.list_closing_pull_requests("octo/widget", 42))
|
||||
assert prs == (100,)
|
||||
assert captured["path"] == "/repos/octo/widget/issues/42/timeline"
|
||||
assert captured["per_page"] == "100"
|
||||
|
||||
|
||||
def test_list_closing_pull_requests_empty_timeline() -> None:
|
||||
transport = httpx.MockTransport(lambda r: httpx.Response(200, json=[]))
|
||||
client = GitHubClient("tok", transport=transport)
|
||||
assert _run_async(client.list_closing_pull_requests("octo/widget", 7)) == ()
|
||||
|
||||
|
||||
def test_list_comment_reactions_filters_to_thumbs_down() -> None:
|
||||
captured: dict[str, str] = {}
|
||||
|
||||
def handler(request: httpx.Request) -> httpx.Response:
|
||||
captured["path"] = request.url.path
|
||||
captured["content"] = request.url.params.get("content", "")
|
||||
captured["per_page"] = request.url.params.get("per_page", "")
|
||||
return httpx.Response(
|
||||
200,
|
||||
json=[
|
||||
{"content": "-1", "user": {"login": "Alice", "type": "User"}},
|
||||
{"content": "-1", "user": {"login": "rando", "type": "User"}},
|
||||
],
|
||||
)
|
||||
|
||||
client = GitHubClient("tok", transport=httpx.MockTransport(handler))
|
||||
reactions = _run_async(client.list_comment_reactions("octo/widget", 999))
|
||||
assert captured["path"] == "/repos/octo/widget/issues/comments/999/reactions"
|
||||
assert captured["content"] == "-1"
|
||||
assert captured["per_page"] == "100"
|
||||
assert tuple(r.user_login for r in reactions) == ("Alice", "rando")
|
||||
assert all(r.content == "-1" for r in reactions)
|
||||
|
||||
|
||||
def test_close_issue_sends_completed_state_reason() -> None:
|
||||
captured: dict[str, object] = {}
|
||||
|
||||
def handler(request: httpx.Request) -> httpx.Response:
|
||||
import json
|
||||
|
||||
captured["method"] = request.method
|
||||
captured["path"] = request.url.path
|
||||
captured["body"] = json.loads(request.content)
|
||||
return httpx.Response(200, json={})
|
||||
|
||||
client = GitHubClient("tok", transport=httpx.MockTransport(handler))
|
||||
assert _run_async(client.close_issue("octo/widget", 42)) is None
|
||||
assert captured["method"] == "PATCH"
|
||||
assert captured["path"] == "/repos/octo/widget/issues/42"
|
||||
assert captured["body"] == {"state": "closed", "state_reason": "completed"}
|
||||
|
||||
|
||||
def test_close_issue_propagates_error() -> None:
|
||||
transport = httpx.MockTransport(lambda r: httpx.Response(404, json={"message": "Not Found"}))
|
||||
client = GitHubClient("tok", transport=transport)
|
||||
with pytest.raises(GitHubError) as exc:
|
||||
_run_async(client.close_issue("octo/widget", 42))
|
||||
assert exc.value.status == 404
|
||||
@@ -0,0 +1,736 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import hashlib
|
||||
import hmac
|
||||
|
||||
from robomp.github_events import (
|
||||
extract_mention,
|
||||
is_maintainer,
|
||||
rate_limit_cap,
|
||||
route,
|
||||
verify_signature,
|
||||
)
|
||||
|
||||
ALLOWLIST = frozenset({"octo/widget"})
|
||||
BOT = "robomp-bot"
|
||||
|
||||
|
||||
def test_verify_signature_positive() -> None:
|
||||
secret = "shh"
|
||||
body = b'{"x":1}'
|
||||
sig = hmac.new(secret.encode(), body, hashlib.sha256).hexdigest()
|
||||
assert verify_signature(secret, body, f"sha256={sig}")
|
||||
|
||||
|
||||
def test_verify_signature_rejects_missing_header() -> None:
|
||||
assert not verify_signature("shh", b"{}", None)
|
||||
assert not verify_signature("shh", b"{}", "")
|
||||
assert not verify_signature("shh", b"{}", "md5=deadbeef")
|
||||
|
||||
|
||||
def test_verify_signature_rejects_wrong_secret() -> None:
|
||||
body = b'{"x":1}'
|
||||
sig = hmac.new(b"right", body, hashlib.sha256).hexdigest()
|
||||
assert not verify_signature("wrong", body, f"sha256={sig}")
|
||||
|
||||
|
||||
def test_route_issue_opened_queues_triage() -> None:
|
||||
decision = route(
|
||||
"issues",
|
||||
{
|
||||
"action": "opened",
|
||||
"issue": {"number": 4, "user": {"login": "alice"}},
|
||||
"repository": {"full_name": "octo/widget"},
|
||||
},
|
||||
allowlist=ALLOWLIST,
|
||||
bot_login=BOT,
|
||||
)
|
||||
assert decision.should_queue
|
||||
assert decision.task == "triage_issue"
|
||||
assert decision.issue_key == "octo/widget#4"
|
||||
|
||||
|
||||
def test_route_skips_disallowed_repo() -> None:
|
||||
decision = route(
|
||||
"issues",
|
||||
{"action": "opened", "issue": {"number": 1}, "repository": {"full_name": "other/repo"}},
|
||||
allowlist=ALLOWLIST,
|
||||
bot_login=BOT,
|
||||
)
|
||||
assert not decision.should_queue
|
||||
assert "allowlist" in decision.reason
|
||||
|
||||
|
||||
def test_route_skips_self_comment() -> None:
|
||||
decision = route(
|
||||
"issue_comment",
|
||||
{
|
||||
"action": "created",
|
||||
"comment": {"user": {"login": BOT}, "body": "hi"},
|
||||
"issue": {"number": 4},
|
||||
"repository": {"full_name": "octo/widget"},
|
||||
},
|
||||
allowlist=ALLOWLIST,
|
||||
bot_login=BOT,
|
||||
)
|
||||
assert not decision.should_queue
|
||||
|
||||
|
||||
def test_route_skips_bot_suffix_comment() -> None:
|
||||
decision = route(
|
||||
"issue_comment",
|
||||
{
|
||||
"action": "created",
|
||||
"comment": {"user": {"login": "github-actions[bot]", "type": "Bot"}, "body": "ci ran"},
|
||||
"issue": {"number": 4},
|
||||
"repository": {"full_name": "octo/widget"},
|
||||
},
|
||||
allowlist=ALLOWLIST,
|
||||
bot_login=BOT,
|
||||
)
|
||||
assert not decision.should_queue
|
||||
assert "bot" in decision.reason
|
||||
|
||||
|
||||
def test_route_skips_user_type_bot() -> None:
|
||||
decision = route(
|
||||
"issue_comment",
|
||||
{
|
||||
"action": "created",
|
||||
"comment": {"user": {"login": "renovate", "type": "Bot"}, "body": "deps"},
|
||||
"issue": {"number": 4},
|
||||
"repository": {"full_name": "octo/widget"},
|
||||
},
|
||||
allowlist=ALLOWLIST,
|
||||
bot_login=BOT,
|
||||
)
|
||||
assert not decision.should_queue
|
||||
|
||||
|
||||
def test_route_comment_routes_handle_comment() -> None:
|
||||
decision = route(
|
||||
"issue_comment",
|
||||
{
|
||||
"action": "created",
|
||||
"comment": {"user": {"login": "alice"}, "body": "hi"},
|
||||
"issue": {"number": 4},
|
||||
"repository": {"full_name": "octo/widget"},
|
||||
},
|
||||
allowlist=ALLOWLIST,
|
||||
bot_login=BOT,
|
||||
)
|
||||
assert decision.should_queue
|
||||
assert decision.task == "handle_comment"
|
||||
assert decision.issue_key == "octo/widget#4"
|
||||
|
||||
|
||||
def test_route_pr_conversation_uses_handle_pr_conversation() -> None:
|
||||
"""A regular comment on a PR (not a review) must NOT route to handle_review."""
|
||||
decision = route(
|
||||
"issue_comment",
|
||||
{
|
||||
"action": "created",
|
||||
"comment": {"user": {"login": "alice"}, "body": "looks good"},
|
||||
"issue": {"number": 9, "pull_request": {"url": "x"}},
|
||||
"repository": {"full_name": "octo/widget"},
|
||||
},
|
||||
allowlist=ALLOWLIST,
|
||||
bot_login=BOT,
|
||||
resolve_issue_from_pr=lambda _r, _n: "octo/widget#42",
|
||||
)
|
||||
assert decision.should_queue
|
||||
assert decision.task == "handle_pr_conversation"
|
||||
|
||||
|
||||
def test_route_pr_conversation_uses_resolver_for_inflight_key() -> None:
|
||||
"""PR-derived events MUST serialize on the originating issue's key."""
|
||||
|
||||
def resolver(repo: str, pr_number: int) -> str | None:
|
||||
assert repo == "octo/widget"
|
||||
assert pr_number == 9
|
||||
return "octo/widget#42"
|
||||
|
||||
decision = route(
|
||||
"issue_comment",
|
||||
{
|
||||
"action": "created",
|
||||
"comment": {"user": {"login": "alice"}, "body": "looks good"},
|
||||
"issue": {"number": 9, "pull_request": {"url": "x"}},
|
||||
"repository": {"full_name": "octo/widget"},
|
||||
},
|
||||
allowlist=ALLOWLIST,
|
||||
bot_login=BOT,
|
||||
resolve_issue_from_pr=resolver,
|
||||
)
|
||||
assert decision.should_queue
|
||||
# Same key as if the user had commented on issue #42 directly.
|
||||
assert decision.issue_key == "octo/widget#42"
|
||||
|
||||
|
||||
def test_route_pr_conversation_falls_back_to_pr_key_when_resolver_misses() -> None:
|
||||
"""Unmapped PR comments still queue so the worker can recover from the PR branch."""
|
||||
|
||||
decision = route(
|
||||
"issue_comment",
|
||||
{
|
||||
"action": "created",
|
||||
"comment": {"user": {"login": "alice"}, "body": "hi"},
|
||||
"issue": {"number": 9, "pull_request": {"url": "x"}},
|
||||
"repository": {"full_name": "octo/widget"},
|
||||
},
|
||||
allowlist=ALLOWLIST,
|
||||
bot_login=BOT,
|
||||
resolve_issue_from_pr=lambda _r, _n: None,
|
||||
)
|
||||
assert decision.should_queue
|
||||
assert decision.task == "handle_pr_conversation"
|
||||
assert decision.submitter == "alice"
|
||||
assert decision.issue_key == "octo/widget#9"
|
||||
|
||||
|
||||
def test_route_review_only_for_bot_authored_pr() -> None:
|
||||
decision = route(
|
||||
"pull_request_review_comment",
|
||||
{
|
||||
"action": "created",
|
||||
"comment": {"user": {"login": "alice"}, "body": "nit"},
|
||||
"pull_request": {"number": 9, "user": {"login": BOT}},
|
||||
"repository": {"full_name": "octo/widget"},
|
||||
},
|
||||
allowlist=ALLOWLIST,
|
||||
bot_login=BOT,
|
||||
resolve_issue_from_pr=lambda _r, _n: "octo/widget#42",
|
||||
)
|
||||
assert decision.should_queue
|
||||
assert decision.task == "handle_review"
|
||||
assert decision.issue_key == "octo/widget#42"
|
||||
|
||||
not_ours = route(
|
||||
"pull_request_review_comment",
|
||||
{
|
||||
"action": "created",
|
||||
"comment": {"user": {"login": "alice"}, "body": "nit"},
|
||||
"pull_request": {"number": 9, "user": {"login": "someone-else"}},
|
||||
"repository": {"full_name": "octo/widget"},
|
||||
},
|
||||
allowlist=ALLOWLIST,
|
||||
bot_login=BOT,
|
||||
)
|
||||
assert not not_ours.should_queue
|
||||
|
||||
|
||||
def test_route_review_comment_falls_back_to_pr_key_when_resolver_misses() -> None:
|
||||
decision = route(
|
||||
"pull_request_review_comment",
|
||||
{
|
||||
"action": "created",
|
||||
"comment": {"user": {"login": "alice"}, "body": "nit"},
|
||||
"pull_request": {"number": 9, "user": {"login": BOT}},
|
||||
"repository": {"full_name": "octo/widget"},
|
||||
},
|
||||
allowlist=ALLOWLIST,
|
||||
bot_login=BOT,
|
||||
resolve_issue_from_pr=lambda _r, _n: None,
|
||||
)
|
||||
assert decision.should_queue
|
||||
assert decision.task == "handle_review"
|
||||
assert decision.submitter == "alice"
|
||||
assert decision.issue_key == "octo/widget#9"
|
||||
|
||||
|
||||
def test_route_pr_closed_only_when_merged_by_bot() -> None:
|
||||
payload = {
|
||||
"action": "closed",
|
||||
"pull_request": {"number": 9, "user": {"login": BOT}, "merged": True},
|
||||
"repository": {"full_name": "octo/widget"},
|
||||
}
|
||||
decision = route(
|
||||
"pull_request",
|
||||
payload,
|
||||
allowlist=ALLOWLIST,
|
||||
bot_login=BOT,
|
||||
resolve_issue_from_pr=lambda _r, _n: "octo/widget#42",
|
||||
)
|
||||
assert decision.should_queue
|
||||
assert decision.task == "cleanup_workspace"
|
||||
assert decision.issue_key == "octo/widget#42"
|
||||
|
||||
fallback = route(
|
||||
"pull_request",
|
||||
payload,
|
||||
allowlist=ALLOWLIST,
|
||||
bot_login=BOT,
|
||||
resolve_issue_from_pr=lambda _r, _n: None,
|
||||
)
|
||||
assert fallback.should_queue
|
||||
assert fallback.task == "cleanup_workspace"
|
||||
assert fallback.issue_key == "octo/widget#9"
|
||||
assert fallback.submitter is None
|
||||
|
||||
payload["pull_request"]["merged"] = False # type: ignore[index]
|
||||
assert not route("pull_request", payload, allowlist=ALLOWLIST, bot_login=BOT).should_queue
|
||||
|
||||
|
||||
def test_route_skips_pull_request_issues_event() -> None:
|
||||
decision = route(
|
||||
"issues",
|
||||
{
|
||||
"action": "opened",
|
||||
"issue": {"number": 4, "pull_request": {"url": "x"}},
|
||||
"repository": {"full_name": "octo/widget"},
|
||||
},
|
||||
allowlist=ALLOWLIST,
|
||||
bot_login=BOT,
|
||||
)
|
||||
assert not decision.should_queue
|
||||
|
||||
|
||||
def test_route_issue_opened_captures_submitter() -> None:
|
||||
decision = route(
|
||||
"issues",
|
||||
{
|
||||
"action": "opened",
|
||||
"issue": {
|
||||
"number": 4,
|
||||
"user": {"login": "alice"},
|
||||
"author_association": "FIRST_TIME_CONTRIBUTOR",
|
||||
},
|
||||
"repository": {"full_name": "octo/widget"},
|
||||
},
|
||||
allowlist=ALLOWLIST,
|
||||
bot_login=BOT,
|
||||
)
|
||||
assert decision.submitter == "alice"
|
||||
assert decision.association == "FIRST_TIME_CONTRIBUTOR"
|
||||
|
||||
|
||||
def test_route_comment_captures_comment_author_association() -> None:
|
||||
decision = route(
|
||||
"issue_comment",
|
||||
{
|
||||
"action": "created",
|
||||
"comment": {
|
||||
"user": {"login": "bob"},
|
||||
"body": "hi",
|
||||
"author_association": "CONTRIBUTOR",
|
||||
},
|
||||
"issue": {"number": 4},
|
||||
"repository": {"full_name": "octo/widget"},
|
||||
},
|
||||
allowlist=ALLOWLIST,
|
||||
bot_login=BOT,
|
||||
)
|
||||
assert decision.submitter == "bob"
|
||||
assert decision.association == "CONTRIBUTOR"
|
||||
|
||||
|
||||
def test_route_pr_merged_carries_no_submitter() -> None:
|
||||
"""Lifecycle events (cleanup on merge) are not user submissions."""
|
||||
payload = {
|
||||
"action": "closed",
|
||||
"pull_request": {"number": 9, "user": {"login": BOT}, "merged": True},
|
||||
"repository": {"full_name": "octo/widget"},
|
||||
}
|
||||
decision = route(
|
||||
"pull_request",
|
||||
payload,
|
||||
allowlist=ALLOWLIST,
|
||||
bot_login=BOT,
|
||||
resolve_issue_from_pr=lambda _r, _n: "octo/widget#42",
|
||||
)
|
||||
assert decision.should_queue
|
||||
assert decision.submitter is None
|
||||
|
||||
|
||||
def test_rate_limit_cap_unlimited_allowlist_beats_association() -> None:
|
||||
# Even a NONE association is unlimited when login is in the explicit list.
|
||||
assert (
|
||||
rate_limit_cap(
|
||||
"can1357",
|
||||
"NONE",
|
||||
unlimited=frozenset({"can1357"}),
|
||||
default=3,
|
||||
contributor=10,
|
||||
)
|
||||
is None
|
||||
)
|
||||
|
||||
|
||||
def test_rate_limit_cap_unlimited_is_case_insensitive() -> None:
|
||||
assert (
|
||||
rate_limit_cap(
|
||||
"Can1357",
|
||||
None,
|
||||
unlimited=frozenset({"can1357"}),
|
||||
default=3,
|
||||
contributor=10,
|
||||
)
|
||||
is None
|
||||
)
|
||||
|
||||
|
||||
def test_rate_limit_cap_trusted_associations_bypass() -> None:
|
||||
for assoc in ("OWNER", "MEMBER", "COLLABORATOR"):
|
||||
assert (
|
||||
rate_limit_cap(
|
||||
"stranger",
|
||||
assoc,
|
||||
unlimited=frozenset(),
|
||||
default=3,
|
||||
contributor=10,
|
||||
)
|
||||
is None
|
||||
), assoc
|
||||
|
||||
|
||||
def test_rate_limit_cap_contributor_tier() -> None:
|
||||
assert (
|
||||
rate_limit_cap(
|
||||
"alice",
|
||||
"CONTRIBUTOR",
|
||||
unlimited=frozenset(),
|
||||
default=3,
|
||||
contributor=10,
|
||||
)
|
||||
== 10
|
||||
)
|
||||
|
||||
|
||||
def test_rate_limit_cap_default_tier_for_unknown_and_first_timer() -> None:
|
||||
for assoc in (None, "NONE", "FIRST_TIME_CONTRIBUTOR", "FIRST_TIMER"):
|
||||
assert (
|
||||
rate_limit_cap(
|
||||
"alice",
|
||||
assoc,
|
||||
unlimited=frozenset(),
|
||||
default=3,
|
||||
contributor=10,
|
||||
)
|
||||
== 3
|
||||
), assoc
|
||||
|
||||
|
||||
# ---------- mention + directive ----------
|
||||
|
||||
|
||||
def test_extract_mention_returns_body_minus_mention() -> None:
|
||||
assert extract_mention("hey @robomp-bot please look", "robomp-bot") == "hey please look"
|
||||
assert extract_mention("@robomp-bot do X", "robomp-bot") == "do X"
|
||||
|
||||
|
||||
def test_extract_mention_returns_none_without_mention() -> None:
|
||||
assert extract_mention("hello there", "robomp-bot") is None
|
||||
assert extract_mention(None, "robomp-bot") is None
|
||||
assert extract_mention("", "robomp-bot") is None
|
||||
|
||||
|
||||
def test_extract_mention_is_case_insensitive() -> None:
|
||||
assert extract_mention("yo @ROBOMP-BOT", "robomp-bot") == "yo"
|
||||
|
||||
|
||||
def test_extract_mention_respects_hyphen_word_boundary() -> None:
|
||||
# @robomp-bot-helper must NOT match @robomp-bot.
|
||||
assert extract_mention("@robomp-bot-helper hi", "robomp-bot") is None
|
||||
|
||||
|
||||
def test_extract_mention_handles_multiple_occurrences() -> None:
|
||||
assert extract_mention("@robomp-bot one, then @robomp-bot two", "robomp-bot") == "one, then two"
|
||||
|
||||
|
||||
def test_is_maintainer_recognizes_explicit_allowlist() -> None:
|
||||
assert is_maintainer("can1357", None, maintainers=frozenset({"can1357"}))
|
||||
assert is_maintainer("Can1357", "NONE", maintainers=frozenset({"can1357"}))
|
||||
|
||||
|
||||
def test_is_maintainer_recognizes_trusted_associations() -> None:
|
||||
for assoc in ("OWNER", "MEMBER", "COLLABORATOR"):
|
||||
assert is_maintainer("anyone", assoc, maintainers=frozenset()), assoc
|
||||
|
||||
|
||||
def test_is_maintainer_rejects_contributor_and_none() -> None:
|
||||
assert not is_maintainer("alice", "CONTRIBUTOR", maintainers=frozenset())
|
||||
assert not is_maintainer("alice", None, maintainers=frozenset())
|
||||
assert is_maintainer(None, "OWNER", maintainers=frozenset()) # association still wins
|
||||
|
||||
|
||||
def test_route_directive_set_on_issue_comment_when_owner_mentions_bot() -> None:
|
||||
decision = route(
|
||||
"issue_comment",
|
||||
{
|
||||
"action": "created",
|
||||
"comment": {
|
||||
"user": {"login": "can1357"},
|
||||
"author_association": "OWNER",
|
||||
"body": "@robomp-bot please refactor X",
|
||||
},
|
||||
"issue": {"number": 9},
|
||||
"repository": {"full_name": "octo/widget"},
|
||||
},
|
||||
allowlist=ALLOWLIST,
|
||||
bot_login=BOT,
|
||||
)
|
||||
assert decision.should_queue
|
||||
assert decision.directive is True
|
||||
assert decision.directive_body == "please refactor X"
|
||||
assert decision.directive_author == "can1357"
|
||||
|
||||
|
||||
def test_route_directive_set_when_login_in_maintainers_list() -> None:
|
||||
decision = route(
|
||||
"issue_comment",
|
||||
{
|
||||
"action": "created",
|
||||
"comment": {
|
||||
"user": {"login": "can1357"},
|
||||
# No author_association field.
|
||||
"body": "@robomp-bot do it",
|
||||
},
|
||||
"issue": {"number": 9},
|
||||
"repository": {"full_name": "octo/widget"},
|
||||
},
|
||||
allowlist=ALLOWLIST,
|
||||
bot_login=BOT,
|
||||
maintainers=frozenset({"can1357"}),
|
||||
)
|
||||
assert decision.directive is True
|
||||
assert decision.directive_body == "do it"
|
||||
assert decision.directive_author == "can1357"
|
||||
|
||||
|
||||
def test_route_directive_unset_for_random_user_even_with_mention() -> None:
|
||||
decision = route(
|
||||
"issue_comment",
|
||||
{
|
||||
"action": "created",
|
||||
"comment": {
|
||||
"user": {"login": "stranger"},
|
||||
"author_association": "NONE",
|
||||
"body": "@robomp-bot please refactor X",
|
||||
},
|
||||
"issue": {"number": 9},
|
||||
"repository": {"full_name": "octo/widget"},
|
||||
},
|
||||
allowlist=ALLOWLIST,
|
||||
bot_login=BOT,
|
||||
)
|
||||
assert decision.should_queue # comment still routed normally
|
||||
assert decision.directive is False
|
||||
assert decision.directive_body is None
|
||||
|
||||
|
||||
def test_route_directive_unset_for_maintainer_without_mention() -> None:
|
||||
decision = route(
|
||||
"issue_comment",
|
||||
{
|
||||
"action": "created",
|
||||
"comment": {
|
||||
"user": {"login": "can1357"},
|
||||
"author_association": "OWNER",
|
||||
"body": "looks good to me",
|
||||
},
|
||||
"issue": {"number": 9},
|
||||
"repository": {"full_name": "octo/widget"},
|
||||
},
|
||||
allowlist=ALLOWLIST,
|
||||
bot_login=BOT,
|
||||
)
|
||||
assert decision.directive is False
|
||||
|
||||
|
||||
def test_route_directive_set_on_pr_conversation() -> None:
|
||||
decision = route(
|
||||
"issue_comment",
|
||||
{
|
||||
"action": "created",
|
||||
"comment": {
|
||||
"user": {"login": "can1357"},
|
||||
"author_association": "OWNER",
|
||||
"body": "@robomp-bot change the indentation in foo.py",
|
||||
},
|
||||
"issue": {"number": 50, "pull_request": {"url": "x"}},
|
||||
"repository": {"full_name": "octo/widget"},
|
||||
},
|
||||
allowlist=ALLOWLIST,
|
||||
bot_login=BOT,
|
||||
resolve_issue_from_pr=lambda _r, _n: "octo/widget#42",
|
||||
)
|
||||
assert decision.should_queue
|
||||
assert decision.task == "handle_pr_conversation"
|
||||
assert decision.directive is True
|
||||
assert decision.directive_body == "change the indentation in foo.py"
|
||||
|
||||
|
||||
def test_route_directive_set_on_review_comment() -> None:
|
||||
decision = route(
|
||||
"pull_request_review_comment",
|
||||
{
|
||||
"action": "created",
|
||||
"comment": {
|
||||
"user": {"login": "can1357"},
|
||||
"author_association": "OWNER",
|
||||
"body": "@robomp-bot use a generator here",
|
||||
},
|
||||
"pull_request": {"number": 50, "user": {"login": BOT}},
|
||||
"repository": {"full_name": "octo/widget"},
|
||||
},
|
||||
allowlist=ALLOWLIST,
|
||||
bot_login=BOT,
|
||||
resolve_issue_from_pr=lambda _r, _n: "octo/widget#42",
|
||||
)
|
||||
assert decision.should_queue
|
||||
assert decision.task == "handle_review"
|
||||
assert decision.directive is True
|
||||
assert decision.directive_body == "use a generator here"
|
||||
|
||||
|
||||
# ---------- reviewer bots ----------
|
||||
|
||||
|
||||
def test_route_reviewer_bot_comment_is_directive_without_mention() -> None:
|
||||
decision = route(
|
||||
"issue_comment",
|
||||
{
|
||||
"action": "created",
|
||||
"comment": {
|
||||
"user": {"login": "chatgpt-codex-connector", "type": "Bot"},
|
||||
"body": "Found two issues in the diff: ...",
|
||||
},
|
||||
"issue": {"number": 9, "pull_request": {"url": "x"}},
|
||||
"repository": {"full_name": "octo/widget"},
|
||||
},
|
||||
allowlist=ALLOWLIST,
|
||||
bot_login=BOT,
|
||||
reviewer_bots=frozenset({"chatgpt-codex-connector"}),
|
||||
resolve_issue_from_pr=lambda _r, _n: "octo/widget#42",
|
||||
)
|
||||
assert decision.should_queue
|
||||
assert decision.task == "handle_pr_conversation"
|
||||
assert decision.directive is True
|
||||
assert decision.directive_body == "Found two issues in the diff: ..."
|
||||
assert decision.directive_author == "chatgpt-codex-connector"
|
||||
|
||||
|
||||
def test_route_reviewer_bot_review_comment_is_directive() -> None:
|
||||
decision = route(
|
||||
"pull_request_review_comment",
|
||||
{
|
||||
"action": "created",
|
||||
"comment": {
|
||||
"user": {"login": "chatgpt-codex-connector", "type": "Bot"},
|
||||
"body": "This branch leaks memory.",
|
||||
},
|
||||
"pull_request": {"number": 50, "user": {"login": BOT}},
|
||||
"repository": {"full_name": "octo/widget"},
|
||||
},
|
||||
allowlist=ALLOWLIST,
|
||||
bot_login=BOT,
|
||||
reviewer_bots=frozenset({"chatgpt-codex-connector"}),
|
||||
resolve_issue_from_pr=lambda _r, _n: "octo/widget#42",
|
||||
)
|
||||
assert decision.should_queue
|
||||
assert decision.task == "handle_review"
|
||||
assert decision.directive is True
|
||||
assert decision.directive_body == "This branch leaks memory."
|
||||
assert decision.directive_author == "chatgpt-codex-connector"
|
||||
|
||||
|
||||
def test_route_random_bot_still_skipped_when_not_in_reviewer_list() -> None:
|
||||
decision = route(
|
||||
"issue_comment",
|
||||
{
|
||||
"action": "created",
|
||||
"comment": {"user": {"login": "renovate", "type": "Bot"}, "body": "deps"},
|
||||
"issue": {"number": 9},
|
||||
"repository": {"full_name": "octo/widget"},
|
||||
},
|
||||
allowlist=ALLOWLIST,
|
||||
bot_login=BOT,
|
||||
reviewer_bots=frozenset({"chatgpt-codex-connector"}),
|
||||
)
|
||||
assert not decision.should_queue
|
||||
assert "bot" in decision.reason
|
||||
|
||||
|
||||
def test_route_reviewer_bot_login_case_insensitive() -> None:
|
||||
decision = route(
|
||||
"issue_comment",
|
||||
{
|
||||
"action": "created",
|
||||
"comment": {
|
||||
"user": {"login": "ChatGPT-Codex-Connector", "type": "Bot"},
|
||||
"body": "feedback",
|
||||
},
|
||||
"issue": {"number": 9, "pull_request": {"url": "x"}},
|
||||
"repository": {"full_name": "octo/widget"},
|
||||
},
|
||||
allowlist=ALLOWLIST,
|
||||
bot_login=BOT,
|
||||
reviewer_bots=frozenset({"chatgpt-codex-connector"}),
|
||||
resolve_issue_from_pr=lambda _r, _n: "octo/widget#42",
|
||||
)
|
||||
assert decision.directive is True
|
||||
assert decision.directive_author == "chatgpt-codex-connector"
|
||||
|
||||
|
||||
def test_route_directive_strips_pragmas_from_maintainer_comment() -> None:
|
||||
decision = route(
|
||||
"issue_comment",
|
||||
{
|
||||
"action": "created",
|
||||
"comment": {
|
||||
"user": {"login": "can1357"},
|
||||
"author_association": "OWNER",
|
||||
"body": "@robomp-bot /model gpt /thinking low\nrefactor X",
|
||||
},
|
||||
"issue": {"number": 9},
|
||||
"repository": {"full_name": "octo/widget"},
|
||||
},
|
||||
allowlist=ALLOWLIST,
|
||||
bot_login=BOT,
|
||||
)
|
||||
assert decision.directive is True
|
||||
assert decision.directive_body == "refactor X"
|
||||
assert decision.directive_pragmas == (("model", "gpt"), ("thinking", "low"))
|
||||
|
||||
|
||||
def test_route_directive_strips_pragmas_from_reviewer_bot_comment() -> None:
|
||||
decision = route(
|
||||
"issue_comment",
|
||||
{
|
||||
"action": "created",
|
||||
"comment": {
|
||||
"user": {"login": "chatgpt-codex-connector", "type": "Bot"},
|
||||
"body": "/model claude\nLeak in foo()",
|
||||
},
|
||||
"issue": {"number": 9, "pull_request": {"url": "x"}},
|
||||
"repository": {"full_name": "octo/widget"},
|
||||
},
|
||||
allowlist=ALLOWLIST,
|
||||
bot_login=BOT,
|
||||
reviewer_bots=frozenset({"chatgpt-codex-connector"}),
|
||||
resolve_issue_from_pr=lambda _r, _n: "octo/widget#42",
|
||||
)
|
||||
assert decision.directive is True
|
||||
assert decision.directive_body == "Leak in foo()"
|
||||
assert decision.directive_pragmas == (("model", "claude"),)
|
||||
|
||||
|
||||
def test_route_non_directive_comment_carries_no_pragmas() -> None:
|
||||
# Random user pragmas must NOT propagate — only directive comments do.
|
||||
decision = route(
|
||||
"issue_comment",
|
||||
{
|
||||
"action": "created",
|
||||
"comment": {
|
||||
"user": {"login": "stranger"},
|
||||
"author_association": "NONE",
|
||||
"body": "/model gpt\nhello",
|
||||
},
|
||||
"issue": {"number": 9},
|
||||
"repository": {"full_name": "octo/widget"},
|
||||
},
|
||||
allowlist=ALLOWLIST,
|
||||
bot_login=BOT,
|
||||
)
|
||||
assert decision.directive is False
|
||||
assert decision.directive_pragmas == ()
|
||||
File diff suppressed because it is too large
Load Diff
@@ -0,0 +1,414 @@
|
||||
"""Unit tests for `robomp.natives_cache`.
|
||||
|
||||
The module's filesystem operations (hardlink, atomic rename, flock) are
|
||||
exercised against `tmp_path`; nothing here requires a running orchestrator.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import errno
|
||||
import json
|
||||
import os
|
||||
import subprocess
|
||||
import threading
|
||||
import time
|
||||
from pathlib import Path
|
||||
|
||||
import pytest
|
||||
|
||||
from robomp.natives_cache import (
|
||||
CACHE_KEY_PATHS,
|
||||
NativesCache,
|
||||
_atomic_link,
|
||||
compute_key,
|
||||
)
|
||||
|
||||
REPO = "octo/widget"
|
||||
|
||||
|
||||
# ---- repo + workspace fixtures ----
|
||||
|
||||
|
||||
def _git(args: list[str], cwd: Path) -> None:
|
||||
subprocess.run(
|
||||
["git", *args],
|
||||
cwd=str(cwd),
|
||||
check=True,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
env=os.environ
|
||||
| {
|
||||
"GIT_AUTHOR_NAME": "t",
|
||||
"GIT_AUTHOR_EMAIL": "t@t",
|
||||
"GIT_COMMITTER_NAME": "t",
|
||||
"GIT_COMMITTER_EMAIL": "t@t",
|
||||
},
|
||||
)
|
||||
|
||||
|
||||
def _seed_repo(root: Path, *, with_all_inputs: bool = True) -> Path:
|
||||
"""Stand up a minimal repo with the cache-key inputs present.
|
||||
|
||||
When `with_all_inputs=False`, only `Cargo.lock` exists — used to exercise
|
||||
the missing-path code path in `compute_key`.
|
||||
"""
|
||||
root.mkdir(parents=True, exist_ok=True)
|
||||
_git(["init", "--initial-branch=main", str(root)], cwd=root.parent)
|
||||
(root / "Cargo.lock").write_text("# lock v1\n")
|
||||
if with_all_inputs:
|
||||
(root / "Cargo.toml").write_text("[workspace]\nmembers = ['crates/*']\n")
|
||||
(root / "rust-toolchain.toml").write_text('[toolchain]\nchannel = "1.85.0"\n')
|
||||
crates = root / "crates" / "pi-natives"
|
||||
crates.mkdir(parents=True)
|
||||
(crates / "Cargo.toml").write_text('[package]\nname = "pi-natives"\n')
|
||||
(crates / "src.rs").write_text("// source\n")
|
||||
natives = root / "packages" / "natives"
|
||||
natives.mkdir(parents=True)
|
||||
(natives / "package.json").write_text('{"name":"@oh-my-pi/pi-natives"}\n')
|
||||
scripts = natives / "scripts"
|
||||
scripts.mkdir()
|
||||
(scripts / "build-native.ts").write_text("// build script\n")
|
||||
native_dir = natives / "native"
|
||||
native_dir.mkdir()
|
||||
(native_dir / "index.d.ts").write_text("// initial typings\n")
|
||||
_git(["-C", str(root), "add", "."], cwd=root.parent)
|
||||
_git(["-C", str(root), "commit", "-m", "init"], cwd=root.parent)
|
||||
return root
|
||||
|
||||
|
||||
def _populate_built_artifacts(repo_dir: Path, *, body: bytes = b"\x7fELF...native") -> Path:
|
||||
"""Fill `packages/natives/native/` with a complete built-artifact set."""
|
||||
native_dir = repo_dir / "packages" / "natives" / "native"
|
||||
native_dir.mkdir(parents=True, exist_ok=True)
|
||||
(native_dir / "pi_natives.linux-arm64.node").write_bytes(body)
|
||||
(native_dir / "index.d.ts").write_text("export const X: number;\n")
|
||||
(native_dir / "index.js").write_text("export const X = 1;\n")
|
||||
(native_dir / "embedded-addon.js").write_text("export const embeddedAddon = null;\n")
|
||||
return native_dir
|
||||
|
||||
|
||||
# ---- compute_key ----
|
||||
|
||||
|
||||
def test_compute_key_deterministic_across_clones(tmp_path: Path) -> None:
|
||||
a = _seed_repo(tmp_path / "a")
|
||||
b_root = tmp_path / "b"
|
||||
subprocess.run(["git", "clone", str(a), str(b_root)], check=True, capture_output=True, text=True)
|
||||
key_a = compute_key(a, target="linux-arm64")
|
||||
key_b = compute_key(b_root, target="linux-arm64")
|
||||
assert key_a == key_b
|
||||
|
||||
|
||||
def test_compute_key_changes_when_each_input_changes(tmp_path: Path) -> None:
|
||||
base = _seed_repo(tmp_path / "base")
|
||||
base_key = compute_key(base, target="linux-arm64")
|
||||
|
||||
# Touching a file under each key path must shift the key.
|
||||
mutations: dict[str, tuple[str, str]] = {
|
||||
"crates": ("crates/pi-natives/src.rs", "// new comment\n"),
|
||||
"Cargo.lock": ("Cargo.lock", "# lock v2\n"),
|
||||
"Cargo.toml": ("Cargo.toml", "[workspace]\nmembers = ['crates/*', 'extra']\n"),
|
||||
"rust-toolchain.toml": ("rust-toolchain.toml", '[toolchain]\nchannel = "1.86.0"\n'),
|
||||
"packages/natives": ("packages/natives/scripts/build-native.ts", "// edited\n"),
|
||||
}
|
||||
for label, (rel, body) in mutations.items():
|
||||
clone = tmp_path / f"clone-{label.replace('/', '-')}"
|
||||
subprocess.run(
|
||||
["git", "clone", str(base), str(clone)],
|
||||
check=True,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
)
|
||||
target = clone / rel
|
||||
target.parent.mkdir(parents=True, exist_ok=True)
|
||||
target.write_text(body)
|
||||
_git(["-C", str(clone), "add", "."], cwd=clone.parent)
|
||||
_git(["-C", str(clone), "commit", "-m", f"mutate {label}"], cwd=clone.parent)
|
||||
new_key = compute_key(clone, target="linux-arm64")
|
||||
assert new_key != base_key, f"key did not change after mutating {label}"
|
||||
|
||||
|
||||
def test_compute_key_target_triple_changes_key(tmp_path: Path) -> None:
|
||||
repo = _seed_repo(tmp_path / "repo")
|
||||
arm = compute_key(repo, target="linux-arm64")
|
||||
x64 = compute_key(repo, target="linux-x64-modern")
|
||||
assert arm != x64
|
||||
|
||||
|
||||
def test_compute_key_handles_missing_inputs(tmp_path: Path) -> None:
|
||||
"""Missing key paths fold to a fixed null hash → key still deterministic."""
|
||||
repo = _seed_repo(tmp_path / "repo", with_all_inputs=False)
|
||||
# Lock-only repo: should compute without error, and adding a tracked
|
||||
# crates/ subtree shifts the key.
|
||||
key_before = compute_key(repo, target="linux-arm64")
|
||||
crates = repo / "crates" / "pi-natives"
|
||||
crates.mkdir(parents=True)
|
||||
(crates / "lib.rs").write_text("// new\n")
|
||||
_git(["-C", str(repo), "add", "."], cwd=repo.parent)
|
||||
_git(["-C", str(repo), "commit", "-m", "add crates"], cwd=repo.parent)
|
||||
key_after = compute_key(repo, target="linux-arm64")
|
||||
assert key_before != key_after
|
||||
|
||||
|
||||
def test_compute_key_uses_all_documented_paths() -> None:
|
||||
# Sanity contract: the exported path list IS the input set.
|
||||
assert CACHE_KEY_PATHS == (
|
||||
"crates",
|
||||
"Cargo.lock",
|
||||
"Cargo.toml",
|
||||
"rust-toolchain.toml",
|
||||
"packages/natives",
|
||||
)
|
||||
|
||||
|
||||
def test_compute_key_raises_on_non_repo(tmp_path: Path) -> None:
|
||||
with pytest.raises(subprocess.CalledProcessError):
|
||||
compute_key(tmp_path, target="linux-arm64")
|
||||
|
||||
|
||||
# ---- populate / capture ----
|
||||
|
||||
|
||||
def _cache(tmp_path: Path, **kwargs: object) -> NativesCache:
|
||||
return NativesCache(tmp_path / "natives-cache", **kwargs) # type: ignore[arg-type]
|
||||
|
||||
|
||||
def test_populate_workspace_miss_is_noop(tmp_path: Path) -> None:
|
||||
cache = _cache(tmp_path)
|
||||
repo_dir = _seed_repo(tmp_path / "ws" / "repo")
|
||||
native_dir = repo_dir / "packages" / "natives" / "native"
|
||||
before = sorted(p.name for p in native_dir.iterdir())
|
||||
hit = cache.populate_workspace(REPO, "deadbeef" * 8, native_dir)
|
||||
after = sorted(p.name for p in native_dir.iterdir())
|
||||
assert hit is None
|
||||
assert before == after
|
||||
|
||||
|
||||
def test_capture_then_populate_shares_node_inode_but_copies_companions(tmp_path: Path) -> None:
|
||||
cache = _cache(tmp_path)
|
||||
src_repo = _seed_repo(tmp_path / "src" / "repo")
|
||||
native_dir = _populate_built_artifacts(src_repo)
|
||||
key = compute_key(src_repo, target="linux-arm64")
|
||||
stored = cache.capture(REPO, key, native_dir, source_workspace="src__001")
|
||||
assert stored is not None
|
||||
manifest = json.loads((stored / "manifest.json").read_text())
|
||||
assert manifest["key"] == key
|
||||
assert "pi_natives.linux-arm64.node" in manifest["node_files"]
|
||||
|
||||
# Populate a fresh workspace from the same source state.
|
||||
dst_repo = src_repo.parent.parent / "dst" / "repo"
|
||||
dst_repo.mkdir(parents=True)
|
||||
_git(["clone", str(src_repo), str(dst_repo)], cwd=dst_repo.parent)
|
||||
dst_native = dst_repo / "packages" / "natives" / "native"
|
||||
dst_native.mkdir(parents=True, exist_ok=True)
|
||||
hit = cache.populate_workspace(REPO, key, dst_native)
|
||||
assert hit is not None
|
||||
assert {p.name for p in hit.files} >= {
|
||||
"pi_natives.linux-arm64.node",
|
||||
"index.d.ts",
|
||||
"index.js",
|
||||
"embedded-addon.js",
|
||||
}
|
||||
# The `.node` is hardlinked: same inode, nlink ≥ 2.
|
||||
cached_node = stored / "pi_natives.linux-arm64.node"
|
||||
workspace_node = dst_native / "pi_natives.linux-arm64.node"
|
||||
assert cached_node.stat().st_ino == workspace_node.stat().st_ino
|
||||
assert cached_node.stat().st_nlink >= 2
|
||||
# Companions are COPIED (independent inodes): in-place rewrite in the
|
||||
# workspace (gen-enums.ts / installGeneratedBindings open-truncate-write)
|
||||
# MUST NOT mutate the cached copy.
|
||||
for name in ("index.d.ts", "index.js", "embedded-addon.js"):
|
||||
cached_companion = stored / name
|
||||
ws_companion = dst_native / name
|
||||
assert cached_companion.stat().st_ino != ws_companion.stat().st_ino, name
|
||||
original = cached_companion.read_text()
|
||||
ws_companion.write_text("rewritten\n")
|
||||
assert cached_companion.read_text() == original, name
|
||||
|
||||
|
||||
def test_capture_skips_when_artifacts_incomplete(tmp_path: Path) -> None:
|
||||
cache = _cache(tmp_path)
|
||||
repo = _seed_repo(tmp_path / "ws" / "repo")
|
||||
native_dir = repo / "packages" / "natives" / "native"
|
||||
# Only the .node — missing companions → capture refuses.
|
||||
(native_dir / "pi_natives.linux-arm64.node").write_bytes(b"x")
|
||||
assert cache.capture(REPO, "k", native_dir) is None
|
||||
# And no entry was created.
|
||||
assert not cache.entry_dir(REPO, "k").exists()
|
||||
|
||||
|
||||
def test_capture_is_idempotent_under_lock(tmp_path: Path) -> None:
|
||||
"""Two concurrent captures of the same key end with one final entry."""
|
||||
cache = _cache(tmp_path)
|
||||
src_repo = _seed_repo(tmp_path / "src" / "repo")
|
||||
_populate_built_artifacts(src_repo)
|
||||
key = compute_key(src_repo, target="linux-arm64")
|
||||
native_dir = src_repo / "packages" / "natives" / "native"
|
||||
|
||||
results: list[Path | None] = []
|
||||
barrier = threading.Barrier(2)
|
||||
|
||||
def run() -> None:
|
||||
barrier.wait()
|
||||
results.append(cache.capture(REPO, key, native_dir))
|
||||
|
||||
threads = [threading.Thread(target=run) for _ in range(2)]
|
||||
for t in threads:
|
||||
t.start()
|
||||
for t in threads:
|
||||
t.join()
|
||||
# Both calls succeed (one captures, the other recognizes the entry).
|
||||
assert all(isinstance(r, Path) for r in results)
|
||||
# Exactly one final entry directory (no leftover staging).
|
||||
repo_root = cache.repo_root(REPO)
|
||||
final_dirs = [p for p in repo_root.iterdir() if p.is_dir() and not p.name.startswith(".")]
|
||||
assert len(final_dirs) == 1
|
||||
assert final_dirs[0].name == key
|
||||
|
||||
|
||||
def test_populate_cross_device_falls_back_to_copy(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
cache = _cache(tmp_path)
|
||||
src_repo = _seed_repo(tmp_path / "src" / "repo")
|
||||
_populate_built_artifacts(src_repo)
|
||||
key = compute_key(src_repo, target="linux-arm64")
|
||||
cache.capture(REPO, key, src_repo / "packages" / "natives" / "native")
|
||||
|
||||
dst_native = tmp_path / "ws2" / "packages" / "natives" / "native"
|
||||
dst_native.mkdir(parents=True)
|
||||
|
||||
# Simulate cross-device hardlink failure for every os.link call.
|
||||
real_link = os.link
|
||||
|
||||
def fake_link(src, dst, *args, **kwargs): # type: ignore[no-untyped-def]
|
||||
raise OSError(errno.EXDEV, "Cross-device link", str(src))
|
||||
|
||||
monkeypatch.setattr(os, "link", fake_link)
|
||||
try:
|
||||
hit = cache.populate_workspace(REPO, key, dst_native)
|
||||
finally:
|
||||
monkeypatch.setattr(os, "link", real_link)
|
||||
assert hit is not None
|
||||
# Files exist (via copy) but are distinct inodes from the cache.
|
||||
cached_node = cache.entry_dir(REPO, key) / "pi_natives.linux-arm64.node"
|
||||
copied_node = dst_native / "pi_natives.linux-arm64.node"
|
||||
assert copied_node.exists()
|
||||
assert cached_node.stat().st_ino != copied_node.stat().st_ino
|
||||
|
||||
|
||||
def test_populate_replaces_existing_file_atomically(tmp_path: Path) -> None:
|
||||
cache = _cache(tmp_path)
|
||||
src_repo = _seed_repo(tmp_path / "src" / "repo")
|
||||
_populate_built_artifacts(src_repo, body=b"\x7fELF.A")
|
||||
key = compute_key(src_repo, target="linux-arm64")
|
||||
cache.capture(REPO, key, src_repo / "packages" / "natives" / "native")
|
||||
|
||||
dst_native = tmp_path / "dst" / "packages" / "natives" / "native"
|
||||
dst_native.mkdir(parents=True)
|
||||
# Pre-existing stub bytes — populate must replace, not append/error.
|
||||
target = dst_native / "pi_natives.linux-arm64.node"
|
||||
target.write_bytes(b"old-stub")
|
||||
hit = cache.populate_workspace(REPO, key, dst_native)
|
||||
assert hit is not None
|
||||
assert target.read_bytes() == b"\x7fELF.A"
|
||||
|
||||
|
||||
# ---- gc ----
|
||||
|
||||
|
||||
def _stamp_entry(cache: NativesCache, repo: str, key: str, captured_at: float) -> Path:
|
||||
entry = cache.entry_dir(repo, key)
|
||||
entry.mkdir(parents=True, exist_ok=True)
|
||||
(entry / "pi_natives.linux-arm64.node").write_bytes(b"x" * 1024)
|
||||
(entry / "index.d.ts").write_text("")
|
||||
(entry / "index.js").write_text("")
|
||||
(entry / "embedded-addon.js").write_text("")
|
||||
(entry / "manifest.json").write_text(
|
||||
json.dumps({"key": key, "captured_at": captured_at, "node_files": ["pi_natives.linux-arm64.node"]})
|
||||
)
|
||||
return entry
|
||||
|
||||
|
||||
def test_gc_evicts_oldest_beyond_entry_cap(tmp_path: Path) -> None:
|
||||
cache = _cache(tmp_path, max_entries_per_repo=2, max_bytes=0)
|
||||
now = time.time()
|
||||
_stamp_entry(cache, REPO, "k1", now - 300)
|
||||
_stamp_entry(cache, REPO, "k2", now - 200)
|
||||
_stamp_entry(cache, REPO, "k3", now - 100)
|
||||
evicted = cache.gc(REPO)
|
||||
assert evicted == 1
|
||||
remaining = {p.name for p in cache.repo_root(REPO).iterdir() if p.is_dir() and not p.name.startswith(".")}
|
||||
assert remaining == {"k2", "k3"}
|
||||
|
||||
|
||||
def test_gc_evicts_for_byte_cap(tmp_path: Path) -> None:
|
||||
cache = _cache(tmp_path, max_entries_per_repo=8, max_bytes=2500)
|
||||
now = time.time()
|
||||
# Each entry weighs ~1024 bytes (the .node); 3 entries → ~3072 bytes > cap.
|
||||
_stamp_entry(cache, REPO, "k1", now - 300)
|
||||
_stamp_entry(cache, REPO, "k2", now - 200)
|
||||
_stamp_entry(cache, REPO, "k3", now - 100)
|
||||
cache.gc(REPO)
|
||||
remaining = {p.name for p in cache.repo_root(REPO).iterdir() if p.is_dir() and not p.name.startswith(".")}
|
||||
# Oldest evicted; at least one survives.
|
||||
assert "k1" not in remaining
|
||||
assert remaining <= {"k2", "k3"}
|
||||
assert remaining
|
||||
|
||||
|
||||
def test_gc_preserves_workspace_hardlinks(tmp_path: Path) -> None:
|
||||
"""Evicting a cache entry must NOT delete the file from workspaces that
|
||||
hardlinked it — kernel inode refcount keeps the data alive."""
|
||||
cache = _cache(tmp_path, max_entries_per_repo=1, max_bytes=0)
|
||||
now = time.time()
|
||||
entry = _stamp_entry(cache, REPO, "k1", now - 500)
|
||||
_stamp_entry(cache, REPO, "k2", now - 100)
|
||||
# Workspace hardlinks the older entry's .node before GC runs.
|
||||
ws_node = tmp_path / "ws" / "pi_natives.linux-arm64.node"
|
||||
ws_node.parent.mkdir(parents=True)
|
||||
os.link(entry / "pi_natives.linux-arm64.node", ws_node)
|
||||
cache.gc(REPO)
|
||||
assert not entry.exists() # cache directory swept
|
||||
assert ws_node.exists() # workspace file survives via inode refcount
|
||||
assert ws_node.read_bytes() == b"x" * 1024
|
||||
|
||||
|
||||
def test_gc_clears_stale_staging_dirs(tmp_path: Path) -> None:
|
||||
cache = _cache(tmp_path)
|
||||
repo_root = cache.repo_root(REPO)
|
||||
repo_root.mkdir(parents=True)
|
||||
stale = repo_root / ".aabb.tmp.99999"
|
||||
stale.mkdir()
|
||||
(stale / "leaked").write_text("from a crashed capture")
|
||||
cache.gc(REPO)
|
||||
assert not stale.exists()
|
||||
|
||||
|
||||
def test_gc_drops_entry_with_missing_manifest(tmp_path: Path) -> None:
|
||||
cache = _cache(tmp_path)
|
||||
incomplete = cache.entry_dir(REPO, "bogus")
|
||||
incomplete.mkdir(parents=True)
|
||||
(incomplete / "pi_natives.linux-arm64.node").write_bytes(b"x")
|
||||
cache.gc(REPO)
|
||||
assert not incomplete.exists()
|
||||
|
||||
|
||||
def test_lookup_rejects_incomplete_entry(tmp_path: Path) -> None:
|
||||
cache = _cache(tmp_path)
|
||||
entry = cache.entry_dir(REPO, "partial")
|
||||
entry.mkdir(parents=True)
|
||||
(entry / "manifest.json").write_text("{}")
|
||||
# No .node → no hit even though manifest exists.
|
||||
assert cache.lookup(REPO, "partial") is None
|
||||
|
||||
|
||||
# ---- _atomic_link ----
|
||||
|
||||
|
||||
def test_atomic_link_replaces_existing_target(tmp_path: Path) -> None:
|
||||
src = tmp_path / "src"
|
||||
src.write_bytes(b"new")
|
||||
dst = tmp_path / "dst"
|
||||
dst.write_bytes(b"old")
|
||||
_atomic_link(src, dst)
|
||||
assert dst.read_bytes() == b"new"
|
||||
assert dst.stat().st_ino == src.stat().st_ino
|
||||
@@ -0,0 +1,482 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import json
|
||||
import os
|
||||
import platform
|
||||
import shutil
|
||||
import subprocess
|
||||
import tempfile
|
||||
from collections.abc import Iterator
|
||||
from pathlib import Path
|
||||
from typing import cast
|
||||
|
||||
import pytest
|
||||
|
||||
from robomp import host_tools
|
||||
from robomp.db import Database
|
||||
from robomp.github_backend import GitHubBackend
|
||||
from robomp.github_client import IssueInfo, RepoInfo
|
||||
from robomp.natives_cache import NativesCache
|
||||
from robomp.natives_cache import compute_key as natives_compute_key
|
||||
from robomp.sandbox import LocalGitTransport, SandboxManager, Workspace
|
||||
|
||||
pytestmark = pytest.mark.skipif(
|
||||
os.environ.get("ROBOMP_PERMISSION_E2E") != "1",
|
||||
reason="set ROBOMP_PERMISSION_E2E=1 to run slot-permission e2e tests",
|
||||
)
|
||||
|
||||
_SLOT_ONE = 2001
|
||||
_SLOT_TWO = 2002
|
||||
_SHARED_OMP_GID = 2000
|
||||
_AUTHOR_NAME = "robomp-bot"
|
||||
_AUTHOR_EMAIL = "robomp-bot@example.invalid"
|
||||
_REPO = "octo/permission-e2e"
|
||||
|
||||
|
||||
def _require_linux_root_toolchain() -> None:
|
||||
if platform.system() != "Linux" or os.geteuid() != 0:
|
||||
pytest.skip("slot permission e2e tests require Linux root so subprocesses can drop to omp-N UIDs")
|
||||
missing = [cmd for cmd in ("git", "bun", "cargo", "python3") if shutil.which(cmd) is None]
|
||||
if missing:
|
||||
pytest.skip(f"slot permission e2e tests require tools on PATH: {', '.join(missing)}")
|
||||
|
||||
|
||||
def _git(args: list[str], cwd: Path, *, env: dict[str, str] | None = None) -> subprocess.CompletedProcess[str]:
|
||||
return subprocess.run(
|
||||
["git", *args],
|
||||
cwd=str(cwd),
|
||||
check=True,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
env=env,
|
||||
)
|
||||
|
||||
|
||||
def _write_seed_repo(seed: Path) -> None:
|
||||
(seed / "src").mkdir(parents=True)
|
||||
(seed / "crates" / "core" / "src").mkdir(parents=True)
|
||||
(seed / "package.json").write_text(
|
||||
json.dumps(
|
||||
{
|
||||
"name": "permission-e2e",
|
||||
"private": True,
|
||||
"type": "module",
|
||||
"scripts": {
|
||||
"check": "bun run check:ts && cargo check --workspace",
|
||||
"check:ts": "biome check src/index.ts",
|
||||
"fix": "biome check --write --unsafe src/index.ts",
|
||||
},
|
||||
"devDependencies": {"@biomejs/biome": "^2.4.14"},
|
||||
},
|
||||
indent=2,
|
||||
)
|
||||
+ "\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
(seed / ".gitignore").write_text("node_modules/\n", encoding="utf-8")
|
||||
(seed / "src" / "index.ts").write_text("export const answer = 42;\n", encoding="utf-8")
|
||||
(seed / "Cargo.toml").write_text(
|
||||
'[workspace]\nmembers = ["crates/core"]\nresolver = "2"\n',
|
||||
encoding="utf-8",
|
||||
)
|
||||
(seed / "rust-toolchain.toml").write_text(
|
||||
'[toolchain]\nchannel = "stable"\nprofile = "minimal"\n',
|
||||
encoding="utf-8",
|
||||
)
|
||||
(seed / "crates" / "core" / "Cargo.toml").write_text(
|
||||
'[package]\nname = "permission-e2e-core"\nversion = "0.1.0"\nedition = "2021"\n\n[lib]\npath = "src/lib.rs"\n',
|
||||
encoding="utf-8",
|
||||
)
|
||||
(seed / "crates" / "core" / "src" / "lib.rs").write_text(
|
||||
"pub fn answer() -> u32 {\n 42\n}\n",
|
||||
encoding="utf-8",
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def slot_tmp_path() -> Iterator[Path]:
|
||||
root = Path(tempfile.mkdtemp(prefix="robomp-permission-e2e-", dir="/tmp"))
|
||||
root.chmod(0o755)
|
||||
try:
|
||||
yield root
|
||||
finally:
|
||||
shutil.rmtree(root, ignore_errors=True)
|
||||
|
||||
|
||||
def _share_tree_with_slots(path: Path) -> None:
|
||||
for root, dirs, files in os.walk(path):
|
||||
root_path = Path(root)
|
||||
os.chown(root_path, 0, _SHARED_OMP_GID)
|
||||
root_path.chmod(0o2770)
|
||||
for dirname in dirs:
|
||||
child = root_path / dirname
|
||||
os.chown(child, 0, _SHARED_OMP_GID)
|
||||
child.chmod(0o2770)
|
||||
for filename in files:
|
||||
child = root_path / filename
|
||||
executable = child.stat().st_mode & 0o111
|
||||
os.chown(child, 0, _SHARED_OMP_GID)
|
||||
child.chmod(0o770 if executable else 0o660)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def upstream_repo(slot_tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path:
|
||||
upstream = slot_tmp_path / "upstream.git"
|
||||
seed = slot_tmp_path / "seed"
|
||||
seed.mkdir()
|
||||
_write_seed_repo(seed)
|
||||
|
||||
_git(["init", "--initial-branch=main", "--bare", str(upstream)], cwd=slot_tmp_path)
|
||||
_git(["init", "--initial-branch=main", str(seed)], cwd=slot_tmp_path)
|
||||
_git(["-C", str(seed), "add", "."], cwd=slot_tmp_path)
|
||||
commit_env = os.environ | {
|
||||
"GIT_AUTHOR_NAME": "seed",
|
||||
"GIT_AUTHOR_EMAIL": "seed@example.invalid",
|
||||
"GIT_COMMITTER_NAME": "seed",
|
||||
"GIT_COMMITTER_EMAIL": "seed@example.invalid",
|
||||
}
|
||||
_git(["-C", str(seed), "commit", "-m", "seed"], cwd=slot_tmp_path, env=commit_env)
|
||||
_git(["-C", str(seed), "remote", "add", "origin", str(upstream)], cwd=slot_tmp_path)
|
||||
_git(["-C", str(seed), "push", "origin", "main"], cwd=slot_tmp_path)
|
||||
_share_tree_with_slots(upstream)
|
||||
git_system_config = slot_tmp_path / "git-system.conf"
|
||||
_git(["config", "--file", str(git_system_config), "--add", "safe.directory", str(upstream)], cwd=slot_tmp_path)
|
||||
git_system_config.chmod(0o644)
|
||||
monkeypatch.setenv("GIT_CONFIG_SYSTEM", str(git_system_config))
|
||||
return upstream
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def tool_loop() -> Iterator[asyncio.AbstractEventLoop]:
|
||||
loop = asyncio.new_event_loop()
|
||||
try:
|
||||
yield loop
|
||||
finally:
|
||||
loop.close()
|
||||
|
||||
|
||||
def _ensure_workspace(
|
||||
root: Path, upstream: Path, *, number: int, slot_uid: int, existing_branch: str | None = None
|
||||
) -> Workspace:
|
||||
manager = SandboxManager(root, transport=LocalGitTransport(token=None))
|
||||
return manager.ensure_workspace(
|
||||
repo=_REPO,
|
||||
number=number,
|
||||
title="permission e2e",
|
||||
clone_url=str(upstream),
|
||||
default_branch="main",
|
||||
existing_branch=existing_branch,
|
||||
author_name=_AUTHOR_NAME,
|
||||
author_email=_AUTHOR_EMAIL,
|
||||
slot_uid=slot_uid,
|
||||
)
|
||||
|
||||
|
||||
def _bindings(
|
||||
*,
|
||||
db: Database,
|
||||
tool_loop: asyncio.AbstractEventLoop,
|
||||
workspace: Workspace,
|
||||
upstream: Path,
|
||||
slot_uid: int,
|
||||
) -> host_tools.ToolBindings:
|
||||
repo = RepoInfo(full_name=_REPO, default_branch="main", clone_url=str(upstream), private=False)
|
||||
issue = IssueInfo(
|
||||
repo=_REPO,
|
||||
number=workspace.issue_number,
|
||||
title="permission e2e",
|
||||
body="",
|
||||
state="open",
|
||||
author="human",
|
||||
labels=(),
|
||||
is_pull_request=False,
|
||||
)
|
||||
return host_tools.ToolBindings(
|
||||
db=db,
|
||||
github=cast(GitHubBackend, object()), # not used by these local-only host-tool paths
|
||||
git_transport=LocalGitTransport(token=None),
|
||||
repo=repo,
|
||||
issue=issue,
|
||||
workspace=workspace,
|
||||
loop=tool_loop,
|
||||
author_name=_AUTHOR_NAME,
|
||||
author_email=_AUTHOR_EMAIL,
|
||||
slot_uid=slot_uid,
|
||||
)
|
||||
|
||||
|
||||
def _run_ok(
|
||||
bindings: host_tools.ToolBindings,
|
||||
cmd: list[str] | tuple[str, ...],
|
||||
*,
|
||||
timeout: float = 180.0,
|
||||
) -> subprocess.CompletedProcess[str]:
|
||||
proc = host_tools._run_repo_command(bindings, cmd, timeout=timeout)
|
||||
assert proc.returncode == 0, (
|
||||
f"command failed as slot {bindings.slot_uid}: {' '.join(cmd)}\nstdout:\n{proc.stdout}\nstderr:\n{proc.stderr}"
|
||||
)
|
||||
return proc
|
||||
|
||||
|
||||
def _write_as_slot(bindings: host_tools.ToolBindings, relative_path: str, content: str) -> None:
|
||||
_run_ok(
|
||||
bindings,
|
||||
[
|
||||
"python3",
|
||||
"-c",
|
||||
(
|
||||
"from pathlib import Path; "
|
||||
"Path(__import__('sys').argv[1]).parent.mkdir(parents=True, exist_ok=True); "
|
||||
"Path(__import__('sys').argv[1]).write_text(__import__('sys').argv[2], encoding='utf-8')"
|
||||
),
|
||||
relative_path,
|
||||
content,
|
||||
],
|
||||
)
|
||||
|
||||
|
||||
def _prepare_shared_cargo_cache(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> Path:
|
||||
cargo_home = tmp_path / "shared-cache" / "cargo"
|
||||
cargo_target = tmp_path / "shared-cache" / "cargo-target"
|
||||
for path in (cargo_home, cargo_target):
|
||||
path.mkdir(parents=True)
|
||||
os.chown(path, 0, _SHARED_OMP_GID)
|
||||
path.chmod(0o2770)
|
||||
monkeypatch.setenv("CARGO_HOME", str(cargo_home))
|
||||
monkeypatch.setenv("CARGO_TARGET_DIR", str(cargo_target))
|
||||
return cargo_target
|
||||
|
||||
|
||||
def test_slot_workspace_runs_bun_biome_cargo_and_git_after_root_reentry(
|
||||
slot_tmp_path: Path,
|
||||
upstream_repo: Path,
|
||||
db: Database,
|
||||
tool_loop: asyncio.AbstractEventLoop,
|
||||
monkeypatch: pytest.MonkeyPatch,
|
||||
) -> None:
|
||||
_require_linux_root_toolchain()
|
||||
cargo_target = _prepare_shared_cargo_cache(slot_tmp_path, monkeypatch)
|
||||
workspaces = slot_tmp_path / "workspaces"
|
||||
|
||||
first = _ensure_workspace(workspaces, upstream_repo, number=101, slot_uid=_SLOT_ONE)
|
||||
stale_bun_cache = first.root / ".omp-xdg" / "cache" / "bun-install" / "root-owned-stale"
|
||||
stale_bun_cache.mkdir(parents=True, exist_ok=True)
|
||||
stale_marker = stale_bun_cache / "marker.txt"
|
||||
stale_marker.write_text("root-owned\n", encoding="utf-8")
|
||||
stale_bun_cache.chmod(0o700)
|
||||
stale_marker.chmod(0o600)
|
||||
|
||||
workspace = _ensure_workspace(
|
||||
workspaces,
|
||||
upstream_repo,
|
||||
number=101,
|
||||
slot_uid=_SLOT_ONE,
|
||||
existing_branch=first.branch,
|
||||
)
|
||||
bindings = _bindings(db=db, tool_loop=tool_loop, workspace=workspace, upstream=upstream_repo, slot_uid=_SLOT_ONE)
|
||||
|
||||
_run_ok(bindings, ["bun", "install", "--no-progress"], timeout=300.0)
|
||||
_run_ok(bindings, ["bun", "run", "check:ts"], timeout=180.0)
|
||||
_run_ok(bindings, ["cargo", "check", "--workspace"], timeout=600.0)
|
||||
host_tools._run_pre_publish_bun_check(bindings, {}, tool_name="gh_push_branch", stage="push")
|
||||
|
||||
runtime_env = host_tools._repo_command_env(bindings)
|
||||
bun_cache = Path(runtime_env["BUN_INSTALL_CACHE_DIR"])
|
||||
assert bun_cache.is_dir()
|
||||
assert bun_cache.stat().st_uid == _SLOT_ONE
|
||||
assert stale_marker.stat().st_uid == _SLOT_ONE
|
||||
assert (cargo_target / "debug").is_dir()
|
||||
assert (cargo_target / "debug").stat().st_gid == _SHARED_OMP_GID
|
||||
|
||||
_write_as_slot(bindings, "src/slot-generated.ts", "export const generatedBySlot = true;\n")
|
||||
_run_ok(bindings, ["git", "add", "src/slot-generated.ts", "Cargo.lock", "bun.lock"])
|
||||
_run_ok(bindings, ["git", "commit", "-m", "slot generated file"])
|
||||
status = _run_ok(bindings, ["git", "status", "--porcelain", "--untracked-files=normal"])
|
||||
assert status.stdout.strip() == ""
|
||||
|
||||
|
||||
def test_git_pool_metadata_survives_root_push_and_retry_slot(
|
||||
slot_tmp_path: Path,
|
||||
upstream_repo: Path,
|
||||
db: Database,
|
||||
tool_loop: asyncio.AbstractEventLoop,
|
||||
) -> None:
|
||||
_require_linux_root_toolchain()
|
||||
workspaces = slot_tmp_path / "workspaces"
|
||||
|
||||
first = _ensure_workspace(workspaces, upstream_repo, number=102, slot_uid=_SLOT_ONE)
|
||||
first_bindings = _bindings(db=db, tool_loop=tool_loop, workspace=first, upstream=upstream_repo, slot_uid=_SLOT_ONE)
|
||||
_write_as_slot(first_bindings, "src/first-slot.ts", "export const firstSlot = 1;\n")
|
||||
_run_ok(first_bindings, ["git", "add", "src/first-slot.ts"])
|
||||
_run_ok(first_bindings, ["git", "commit", "-m", "first slot commit"])
|
||||
|
||||
first_head = host_tools._guarded_push_branch(first_bindings, {}, "gh_push_branch", first.branch)
|
||||
remote_head = _git(["--git-dir", str(upstream_repo), "rev-parse", first.branch], cwd=slot_tmp_path).stdout.strip()
|
||||
assert remote_head == first_head
|
||||
|
||||
retry = _ensure_workspace(
|
||||
workspaces,
|
||||
upstream_repo,
|
||||
number=102,
|
||||
slot_uid=_SLOT_TWO,
|
||||
existing_branch=first.branch,
|
||||
)
|
||||
retry_bindings = _bindings(db=db, tool_loop=tool_loop, workspace=retry, upstream=upstream_repo, slot_uid=_SLOT_TWO)
|
||||
|
||||
_run_ok(retry_bindings, ["git", "fsck", "--no-progress"], timeout=180.0)
|
||||
_write_as_slot(retry_bindings, "src/retry-slot.ts", "export const retrySlot = 2;\n")
|
||||
_run_ok(retry_bindings, ["git", "add", "src/retry-slot.ts"])
|
||||
_run_ok(retry_bindings, ["git", "commit", "-m", "retry slot commit"])
|
||||
|
||||
retry_head = host_tools._guarded_push_branch(retry_bindings, {}, "gh_push_branch", retry.branch)
|
||||
remote_retry_head = _git(
|
||||
["--git-dir", str(upstream_repo), "rev-parse", retry.branch], cwd=slot_tmp_path
|
||||
).stdout.strip()
|
||||
assert remote_retry_head == retry_head
|
||||
assert retry_head != first_head
|
||||
|
||||
|
||||
def _prepare_shared_natives_cache(slot_tmp_path: Path) -> NativesCache:
|
||||
"""Provision `/data/cache/pi-natives` shape (root:omp, setgid 2770)."""
|
||||
cache_root = slot_tmp_path / "cache" / "pi-natives"
|
||||
cache_root.mkdir(parents=True)
|
||||
os.chown(cache_root, 0, _SHARED_OMP_GID)
|
||||
cache_root.chmod(0o2770)
|
||||
return NativesCache(cache_root)
|
||||
|
||||
|
||||
def _stage_built_natives(bindings: host_tools.ToolBindings, *, body: str = "ELFx") -> None:
|
||||
"""Mirror what a napi build would leave in `packages/natives/native/`.
|
||||
|
||||
Writes the four cached files AS THE SLOT so ownership matches a real
|
||||
post-build workspace; capture pulls these into the cache.
|
||||
"""
|
||||
_write_as_slot(bindings, "packages/natives/native/pi_natives.linux-arm64.node", body)
|
||||
_write_as_slot(bindings, "packages/natives/native/index.d.ts", "export const X: number;\n")
|
||||
_write_as_slot(bindings, "packages/natives/native/index.js", "export const X = 1;\n")
|
||||
_write_as_slot(
|
||||
bindings,
|
||||
"packages/natives/native/embedded-addon.js",
|
||||
"export const embeddedAddon = null;\n",
|
||||
)
|
||||
|
||||
|
||||
def test_natives_cache_shares_artifacts_across_slot_workspaces(
|
||||
slot_tmp_path: Path,
|
||||
upstream_repo: Path,
|
||||
db: Database,
|
||||
tool_loop: asyncio.AbstractEventLoop,
|
||||
) -> None:
|
||||
"""End-to-end: capture under slot 1, populate under slot 2, prove that:
|
||||
|
||||
1. A capture from a slot-owned workspace lands in the shared cache with
|
||||
group `omp` setgid inheritance so any other slot can read it.
|
||||
2. ensure_workspace under a different slot UID auto-populates the cached
|
||||
`.node` (hardlink, inode shared) and copies the companions.
|
||||
3. Slot 2 can read the populated `.node`, and a temp-rename rebuild
|
||||
(mirroring napi's `installBinary`) leaves the cache entry intact.
|
||||
4. An in-place truncate-rewrite of a companion (mirroring `gen-enums.ts`
|
||||
/ `installGeneratedBindings`) does NOT mutate the cached companion —
|
||||
this is exactly why companions are copied, not hardlinked.
|
||||
"""
|
||||
_require_linux_root_toolchain()
|
||||
workspaces = slot_tmp_path / "workspaces"
|
||||
natives_cache = _prepare_shared_natives_cache(slot_tmp_path)
|
||||
manager = SandboxManager(
|
||||
workspaces,
|
||||
transport=LocalGitTransport(token=None),
|
||||
natives_cache=natives_cache,
|
||||
)
|
||||
|
||||
# --- Workspace 1: stage built artifacts and capture them as the orchestrator. ---
|
||||
ws1 = manager.ensure_workspace(
|
||||
repo=_REPO,
|
||||
number=301,
|
||||
title="natives cache producer",
|
||||
clone_url=str(upstream_repo),
|
||||
default_branch="main",
|
||||
author_name=_AUTHOR_NAME,
|
||||
author_email=_AUTHOR_EMAIL,
|
||||
slot_uid=_SLOT_ONE,
|
||||
)
|
||||
bindings1 = _bindings(db=db, tool_loop=tool_loop, workspace=ws1, upstream=upstream_repo, slot_uid=_SLOT_ONE)
|
||||
_stage_built_natives(bindings1, body="ELFx-original")
|
||||
|
||||
key = natives_compute_key(ws1.repo_dir, target="linux-arm64")
|
||||
native_dir1 = ws1.repo_dir / "packages" / "natives" / "native"
|
||||
stored = natives_cache.capture(_REPO, key, native_dir1, source_workspace=ws1.workspace_key)
|
||||
assert stored is not None
|
||||
cached_node = stored / "pi_natives.linux-arm64.node"
|
||||
cached_companion = stored / "index.d.ts"
|
||||
# Cache root is setgid `omp`; new files inherit gid `omp` so any slot
|
||||
# with `extra_groups=[omp]` can read them.
|
||||
assert cached_node.stat().st_gid == _SHARED_OMP_GID
|
||||
assert cached_companion.stat().st_gid == _SHARED_OMP_GID
|
||||
|
||||
# --- Workspace 2: a different slot UID gets auto-populated on ensure. ---
|
||||
ws2 = manager.ensure_workspace(
|
||||
repo=_REPO,
|
||||
number=302,
|
||||
title="natives cache consumer",
|
||||
clone_url=str(upstream_repo),
|
||||
default_branch="main",
|
||||
author_name=_AUTHOR_NAME,
|
||||
author_email=_AUTHOR_EMAIL,
|
||||
slot_uid=_SLOT_TWO,
|
||||
)
|
||||
bindings2 = _bindings(db=db, tool_loop=tool_loop, workspace=ws2, upstream=upstream_repo, slot_uid=_SLOT_TWO)
|
||||
native_dir2 = ws2.repo_dir / "packages" / "natives" / "native"
|
||||
ws2_node = native_dir2 / "pi_natives.linux-arm64.node"
|
||||
ws2_companion = native_dir2 / "index.d.ts"
|
||||
assert ws2_node.exists(), "auto-populate must hardlink the .node into ws2"
|
||||
assert ws2_companion.exists(), "auto-populate must copy companions into ws2"
|
||||
|
||||
# The .node is hardlinked: same inode, nlink ≥ 2.
|
||||
assert ws2_node.stat().st_ino == cached_node.stat().st_ino
|
||||
assert cached_node.stat().st_nlink >= 2
|
||||
# The companion is COPIED: independent inode.
|
||||
assert ws2_companion.stat().st_ino != cached_companion.stat().st_ino
|
||||
|
||||
# Slot 2 must be able to read the populated artifacts (group omp + 0660
|
||||
# via setgid inheritance from the cache root).
|
||||
_run_ok(bindings2, ["test", "-r", "packages/natives/native/pi_natives.linux-arm64.node"])
|
||||
_run_ok(bindings2, ["test", "-r", "packages/natives/native/index.d.ts"])
|
||||
|
||||
# --- Rebuild simulation: napi's installBinary does temp + rename. ---
|
||||
# Mirrors `fs.copyFile(src, tempPath); fs.rename(tempPath, dest)`.
|
||||
_run_ok(
|
||||
bindings2,
|
||||
[
|
||||
"python3",
|
||||
"-c",
|
||||
(
|
||||
"import os, sys; "
|
||||
"dest = sys.argv[1]; "
|
||||
"tmp = dest + '.tmp.rebuild'; "
|
||||
"open(tmp, 'wb').write(b'REBUILT'); "
|
||||
"os.rename(tmp, dest)"
|
||||
),
|
||||
"packages/natives/native/pi_natives.linux-arm64.node",
|
||||
],
|
||||
)
|
||||
# Workspace sees the rebuilt bytes; cache is untouched (new inode in ws).
|
||||
assert ws2_node.read_bytes() == b"REBUILT"
|
||||
assert cached_node.read_bytes() == b"ELFx-original"
|
||||
assert ws2_node.stat().st_ino != cached_node.stat().st_ino
|
||||
|
||||
# --- Companion-rewrite simulation: gen-enums.ts open-truncate-writes. ---
|
||||
# Mirrors `await Bun.write(jsPath, js)` / Python `Path.write_text`.
|
||||
_write_as_slot(
|
||||
bindings2,
|
||||
"packages/natives/native/index.d.ts",
|
||||
"// regenerated by gen-enums\n",
|
||||
)
|
||||
assert ws2_companion.read_text() == "// regenerated by gen-enums\n"
|
||||
# Cache copy stays at its original content — copies absorbed the rewrite.
|
||||
assert cached_companion.read_text() == "export const X: number;\n"
|
||||
|
||||
# --- Recapture from ws2 (different key now — but same key here since
|
||||
# tree didn't change) is idempotent under the flock. ---
|
||||
again = natives_cache.capture(_REPO, key, native_dir2, source_workspace=ws2.workspace_key)
|
||||
assert again is not None and again == stored, "second capture must reuse the same entry"
|
||||
@@ -0,0 +1,132 @@
|
||||
"""Coverage for the directive prompt assembly."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from dataclasses import dataclass
|
||||
|
||||
from robomp import persona
|
||||
from robomp.worker import DirectiveInfo, ThreadMessage
|
||||
|
||||
|
||||
@dataclass(slots=True, frozen=True)
|
||||
class _Repo:
|
||||
full_name: str = "octo/widget"
|
||||
default_branch: str = "main"
|
||||
clone_url: str = ""
|
||||
private: bool = False
|
||||
|
||||
|
||||
@dataclass(slots=True, frozen=True)
|
||||
class _Issue:
|
||||
repo: str = "octo/widget"
|
||||
number: int = 1080
|
||||
title: str = "broken thing"
|
||||
body: str = "the body text"
|
||||
state: str = "open"
|
||||
author: str = "alice"
|
||||
labels: tuple[str, ...] = ()
|
||||
is_pull_request: bool = False
|
||||
|
||||
|
||||
@dataclass(slots=True, frozen=True)
|
||||
class _Workspace:
|
||||
branch: str = "farm/abc/test"
|
||||
session_dir: str = "/tmp/session"
|
||||
context_dir: str = "/tmp/ctx"
|
||||
repo_dir: str = "/tmp/repo"
|
||||
|
||||
|
||||
@dataclass(slots=True, frozen=True)
|
||||
class _Comment:
|
||||
id: int = 1
|
||||
author: str = "can1357"
|
||||
body: str = "@roboomp please fix"
|
||||
created_at: str = "2026-05-14T20:00:00Z"
|
||||
|
||||
|
||||
def test_render_thread_empty_yields_placeholder() -> None:
|
||||
assert persona._render_thread(()).startswith("(no prior")
|
||||
|
||||
|
||||
def test_render_thread_orders_kinds_with_appropriate_headers() -> None:
|
||||
thread = (
|
||||
ThreadMessage(kind="issue_body", author="alice", body="orig report", created_at=""),
|
||||
ThreadMessage(kind="comment", author="bob", body="me too", created_at="2026-05-01T10:00:00Z"),
|
||||
ThreadMessage(
|
||||
kind="review_comment",
|
||||
author="codex",
|
||||
body="leak here",
|
||||
created_at="2026-05-02T10:00:00Z",
|
||||
path="src/foo.py",
|
||||
line=42,
|
||||
),
|
||||
ThreadMessage(
|
||||
kind="review",
|
||||
author="codex",
|
||||
body="two issues",
|
||||
created_at="2026-05-02T10:01:00Z",
|
||||
state="CHANGES_REQUESTED",
|
||||
),
|
||||
)
|
||||
out = persona._render_thread(thread)
|
||||
# Issue body header (no timestamp).
|
||||
assert "### @alice — issue body" in out
|
||||
assert "orig report" in out
|
||||
# Comment header with timestamp.
|
||||
assert "### @bob — comment *(2026-05-01T10:00:00Z)*" in out
|
||||
assert "me too" in out
|
||||
# Review comment with file:line anchor.
|
||||
assert "### @codex — review comment on `src/foo.py`:L42" in out
|
||||
assert "leak here" in out
|
||||
# Review with state badge.
|
||||
assert "### @codex — review (CHANGES_REQUESTED)" in out
|
||||
assert "two issues" in out
|
||||
|
||||
|
||||
def test_directive_prompt_embeds_thread_and_directive_body() -> None:
|
||||
thread = (
|
||||
ThreadMessage(kind="comment", author="alice", body="follow up please", created_at="2026-05-01T10:00:00Z"),
|
||||
)
|
||||
out = persona.directive(
|
||||
repo=_Repo(),
|
||||
issue=_Issue(),
|
||||
comment=_Comment(),
|
||||
workspace=_Workspace(),
|
||||
directive=DirectiveInfo(body="apply fix Y", author="can1357", thread=thread),
|
||||
pr_status="PR #1080 is open",
|
||||
)
|
||||
assert "Directive on octo/widget#1080" in out
|
||||
assert "@can1357" in out
|
||||
assert "apply fix Y" in out
|
||||
assert "follow up please" in out
|
||||
assert "PR #1080 is open" in out
|
||||
|
||||
|
||||
def test_kickoff_directive_prompt_embeds_thread_and_classify_instruction() -> None:
|
||||
thread = (ThreadMessage(kind="issue_body", author="alice", body="failing on macos", created_at=""),)
|
||||
out = persona.kickoff_directive(
|
||||
repo=_Repo(),
|
||||
issue=_Issue(),
|
||||
workspace=_Workspace(),
|
||||
directive=DirectiveInfo(body="reproduce + fix", author="can1357", thread=thread),
|
||||
)
|
||||
assert "Maintainer directive on octo/widget#1080" in out
|
||||
assert "failing on macos" in out
|
||||
assert "reproduce + fix" in out
|
||||
# The kickoff variant must still tell the agent to classify first.
|
||||
assert "Classify first" in out
|
||||
|
||||
|
||||
def test_resume_triage_renders_branch_and_issue() -> None:
|
||||
out = persona.resume_triage(
|
||||
repo=_Repo(),
|
||||
issue=_Issue(),
|
||||
workspace=_Workspace(),
|
||||
)
|
||||
# Working branch surfaces literally so the agent sees what it's on.
|
||||
assert "farm/abc/test" in out
|
||||
# Issue identity surfaces with the title.
|
||||
assert "octo/widget#1080" in out
|
||||
assert "broken thing" in out
|
||||
# The prompt instructs the agent to reconcile drift via fetch_issue_thread.
|
||||
assert "fetch_issue_thread" in out
|
||||
@@ -0,0 +1,147 @@
|
||||
from __future__ import annotations
|
||||
|
||||
from robomp.pragmas import (
|
||||
parse_pragmas,
|
||||
pragma_value,
|
||||
resolve_model_alias,
|
||||
resolve_thinking_level,
|
||||
)
|
||||
|
||||
|
||||
def test_parse_single_inline_command() -> None:
|
||||
body = "/model gpt\nfix the off-by-one in foo()"
|
||||
cleaned, pragmas = parse_pragmas(body)
|
||||
assert cleaned == "fix the off-by-one in foo()"
|
||||
assert pragmas == (("model", "gpt"),)
|
||||
|
||||
|
||||
def test_parse_multiple_commands_on_one_line() -> None:
|
||||
body = "/model gpt /thinking low\nrun"
|
||||
cleaned, pragmas = parse_pragmas(body)
|
||||
assert cleaned == "run"
|
||||
assert pragmas == (("model", "gpt"), ("thinking", "low"))
|
||||
|
||||
|
||||
def test_parse_stacked_commands() -> None:
|
||||
body = "/model gpt\n/thinking low\nrun"
|
||||
cleaned, pragmas = parse_pragmas(body)
|
||||
assert cleaned == "run"
|
||||
assert pragmas == (("model", "gpt"), ("thinking", "low"))
|
||||
|
||||
|
||||
def test_parse_equals_form() -> None:
|
||||
body = "/model=gpt /thinking=low\nrun"
|
||||
cleaned, pragmas = parse_pragmas(body)
|
||||
assert cleaned == "run"
|
||||
assert pragmas == (("model", "gpt"), ("thinking", "low"))
|
||||
|
||||
|
||||
def test_parse_indented_command_line() -> None:
|
||||
body = " /model gpt\nrun"
|
||||
cleaned, pragmas = parse_pragmas(body)
|
||||
assert cleaned == "run"
|
||||
assert pragmas == (("model", "gpt"),)
|
||||
|
||||
|
||||
def test_mixed_line_is_not_consumed() -> None:
|
||||
# Trailing prose after a command is part of the line — keep the line.
|
||||
body = "/model gpt fix the bug"
|
||||
cleaned, pragmas = parse_pragmas(body)
|
||||
assert cleaned == "/model gpt fix the bug"
|
||||
assert pragmas == ()
|
||||
|
||||
|
||||
def test_path_references_are_not_consumed() -> None:
|
||||
body = "/src/foo.py:42 is the offender\n/model gpt\nfix it"
|
||||
cleaned, pragmas = parse_pragmas(body)
|
||||
assert cleaned == "/src/foo.py:42 is the offender\nfix it"
|
||||
assert pragmas == (("model", "gpt"),)
|
||||
|
||||
|
||||
def test_command_without_value_is_not_consumed() -> None:
|
||||
body = "/model\nrun"
|
||||
cleaned, pragmas = parse_pragmas(body)
|
||||
assert cleaned == "/model\nrun"
|
||||
assert pragmas == ()
|
||||
|
||||
|
||||
def test_dangling_command_aborts_whole_line() -> None:
|
||||
# `/model gpt /thinking` — second command has no value, so the WHOLE line
|
||||
# is left untouched (atomic per-line consumption).
|
||||
body = "/model gpt /thinking\nrun"
|
||||
cleaned, pragmas = parse_pragmas(body)
|
||||
assert cleaned == "/model gpt /thinking\nrun"
|
||||
assert pragmas == ()
|
||||
|
||||
|
||||
def test_preserves_interior_blank_lines_after_strip() -> None:
|
||||
body = "/model gpt\n\nbody one\n\nbody two"
|
||||
cleaned, pragmas = parse_pragmas(body)
|
||||
assert cleaned == "body one\n\nbody two"
|
||||
assert pragmas == (("model", "gpt"),)
|
||||
|
||||
|
||||
def test_empty_body() -> None:
|
||||
cleaned, pragmas = parse_pragmas("")
|
||||
assert cleaned == ""
|
||||
assert pragmas == ()
|
||||
|
||||
|
||||
def test_key_case_normalized_value_preserved() -> None:
|
||||
body = "/MODEL GPT-5.5\nrun"
|
||||
cleaned, pragmas = parse_pragmas(body)
|
||||
assert cleaned == "run"
|
||||
assert pragmas == (("model", "GPT-5.5"),)
|
||||
|
||||
|
||||
def test_pragma_value_last_wins() -> None:
|
||||
assert pragma_value((("model", "a"), ("model", "b")), "model") == "b"
|
||||
assert pragma_value((("model", "a"),), "MODEL") == "a"
|
||||
assert pragma_value((), "model") is None
|
||||
|
||||
|
||||
def test_resolve_model_alias_precedence() -> None:
|
||||
pool = ("p-anthropic/claude-sonnet-4-6", "p-openai/gpt-5.5", "p-openai/gpt-5.5-mini")
|
||||
# Short-name-after-slash beats substring.
|
||||
assert resolve_model_alias("gpt-5.5", pool) == "p-openai/gpt-5.5"
|
||||
# Substring is fallback.
|
||||
assert resolve_model_alias("gpt", pool) == "p-openai/gpt-5.5"
|
||||
assert resolve_model_alias("claude", pool) == "p-anthropic/claude-sonnet-4-6"
|
||||
|
||||
|
||||
def test_resolve_model_alias_full_id() -> None:
|
||||
pool = ("p-openai/gpt-5.5", "p-anthropic/claude-sonnet-4-6")
|
||||
assert resolve_model_alias("p-openai/gpt-5.5", pool) == "p-openai/gpt-5.5"
|
||||
|
||||
|
||||
def test_resolve_model_alias_no_match() -> None:
|
||||
pool = ("p-anthropic/claude-sonnet-4-6",)
|
||||
assert resolve_model_alias("gpt", pool) is None
|
||||
assert resolve_model_alias("", pool) is None
|
||||
|
||||
|
||||
def test_resolve_thinking_level_aliases() -> None:
|
||||
# Spec from the user: xhi|xhigh|hi|high|med|medium|lo|low|none|off|no.
|
||||
assert resolve_thinking_level("off") == "off"
|
||||
assert resolve_thinking_level("none") == "off"
|
||||
assert resolve_thinking_level("no") == "off"
|
||||
assert resolve_thinking_level("lo") == "low"
|
||||
assert resolve_thinking_level("low") == "low"
|
||||
assert resolve_thinking_level("med") == "medium"
|
||||
assert resolve_thinking_level("medium") == "medium"
|
||||
assert resolve_thinking_level("hi") == "high"
|
||||
assert resolve_thinking_level("high") == "high"
|
||||
assert resolve_thinking_level("xhi") == "xhigh"
|
||||
assert resolve_thinking_level("xhigh") == "xhigh"
|
||||
|
||||
|
||||
def test_resolve_thinking_level_case_insensitive() -> None:
|
||||
assert resolve_thinking_level("HIGH") == "high"
|
||||
assert resolve_thinking_level(" Hi ") == "high"
|
||||
assert resolve_thinking_level("XHi") == "xhigh"
|
||||
|
||||
|
||||
def test_resolve_thinking_level_rejects_unknown() -> None:
|
||||
assert resolve_thinking_level("ultra") is None
|
||||
assert resolve_thinking_level("") is None
|
||||
assert resolve_thinking_level("minimal") is None
|
||||
@@ -0,0 +1,566 @@
|
||||
"""Coverage for `GitHubProxyClient` + `ProxyGitTransport` against an
|
||||
ASGI-wrapped proxy app and a hand-rolled `httpx.MockTransport`."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import json
|
||||
import os
|
||||
import subprocess
|
||||
from collections.abc import Callable
|
||||
from pathlib import Path
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
from pydantic import SecretStr
|
||||
|
||||
from robomp.config import Settings
|
||||
from robomp.git_ops import HeadDriftError
|
||||
from robomp.github_client import (
|
||||
CommentInfo,
|
||||
GitHubClient,
|
||||
GitHubError,
|
||||
IssueInfo,
|
||||
IssueSummary,
|
||||
PullRequestInfo,
|
||||
PullRequestReviewInfo,
|
||||
ReactionInfo,
|
||||
RepoInfo,
|
||||
ReviewCommentInfo,
|
||||
)
|
||||
from robomp.proxy.server import create_proxy_app
|
||||
from robomp.proxy_client import GitHubProxyClient, ProxyGitTransport
|
||||
from robomp.proxy_hmac import HEADER_SIGNATURE, HEADER_TIMESTAMP, verify
|
||||
from robomp.sandbox import workspace_key
|
||||
|
||||
_HMAC = "test-hmac-key-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
|
||||
_HMAC_BYTES = _HMAC.encode("utf-8")
|
||||
_TOKEN = "ghp_test_token_value"
|
||||
|
||||
|
||||
# ---------- shared helpers ----------
|
||||
|
||||
|
||||
def _build_settings(tmp_path: Path) -> Settings:
|
||||
cfg = Settings.model_construct(
|
||||
github_token=SecretStr(_TOKEN),
|
||||
github_webhook_secret=SecretStr("webhook-secret"),
|
||||
bot_login="robomp-bot",
|
||||
git_author_email="robomp-bot@example.invalid",
|
||||
repo_allowlist_raw="octo/widget",
|
||||
gh_proxy_url=None,
|
||||
gh_proxy_hmac_key=SecretStr(_HMAC),
|
||||
gh_proxy_bind_host="0.0.0.0",
|
||||
gh_proxy_bind_port=8081,
|
||||
workspace_root=tmp_path / "workspaces",
|
||||
sqlite_path=tmp_path / "robomp.sqlite",
|
||||
log_dir=tmp_path / "logs",
|
||||
)
|
||||
cfg.ensure_paths()
|
||||
return cfg
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def proxy_settings(tmp_path: Path) -> Settings:
|
||||
return _build_settings(tmp_path)
|
||||
|
||||
|
||||
def _git(args: list[str], cwd: Path) -> subprocess.CompletedProcess[str]:
|
||||
return subprocess.run(
|
||||
["git", *args],
|
||||
cwd=str(cwd),
|
||||
check=True,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
env=os.environ
|
||||
| {
|
||||
"GIT_AUTHOR_NAME": "t",
|
||||
"GIT_AUTHOR_EMAIL": "t@t",
|
||||
"GIT_COMMITTER_NAME": "t",
|
||||
"GIT_COMMITTER_EMAIL": "t@t",
|
||||
},
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def upstream_repo(tmp_path: Path) -> Path:
|
||||
repo = tmp_path / "upstream.git"
|
||||
repo.mkdir()
|
||||
_git(["init", "--initial-branch=main", "--bare", str(repo)], tmp_path)
|
||||
seed = tmp_path / "seed"
|
||||
seed.mkdir()
|
||||
_git(["init", "--initial-branch=main", str(seed)], tmp_path)
|
||||
(seed / "README.md").write_text("hello\n", encoding="utf-8")
|
||||
_git(["-C", str(seed), "add", "."], tmp_path)
|
||||
_git(["-C", str(seed), "commit", "-m", "init"], tmp_path)
|
||||
_git(["-C", str(seed), "remote", "add", "origin", str(repo)], tmp_path)
|
||||
_git(["-C", str(seed), "push", "origin", "main"], tmp_path)
|
||||
return repo
|
||||
|
||||
|
||||
def _stage_workspace(cfg: Settings, upstream: Path, repo: str, number: int, branch: str) -> tuple[Path, str]:
|
||||
ws_dir = Path(cfg.workspace_root) / workspace_key(repo, number)
|
||||
ws_dir.mkdir(parents=True, exist_ok=True)
|
||||
repo_dir = ws_dir / "repo"
|
||||
_git(["clone", str(upstream), str(repo_dir)], ws_dir)
|
||||
_git(["-C", str(repo_dir), "config", "user.email", "t@t"], ws_dir)
|
||||
_git(["-C", str(repo_dir), "config", "user.name", "t"], ws_dir)
|
||||
_git(["-C", str(repo_dir), "checkout", "-b", branch], ws_dir)
|
||||
(repo_dir / "x.txt").write_text("x", encoding="utf-8")
|
||||
_git(["-C", str(repo_dir), "add", "."], ws_dir)
|
||||
_git(["-C", str(repo_dir), "commit", "-m", "x"], ws_dir)
|
||||
proc = _git(["-C", str(repo_dir), "rev-parse", "HEAD"], ws_dir)
|
||||
return repo_dir, proc.stdout.strip()
|
||||
|
||||
|
||||
def _bare_has_branch(bare: Path, branch: str) -> bool:
|
||||
proc = subprocess.run(
|
||||
["git", "-C", str(bare), "branch", "--list", branch],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=False,
|
||||
)
|
||||
return bool(proc.stdout.strip())
|
||||
|
||||
|
||||
def _attach_gh(app, handler: Callable[[httpx.Request], httpx.Response]) -> None:
|
||||
app.state.github = GitHubClient(_TOKEN, transport=httpx.MockTransport(handler))
|
||||
|
||||
|
||||
# Sync httpx.Client cannot accept httpx.ASGITransport (which is async-only).
|
||||
# Bridge by running the async transport inside a one-shot event loop per call.
|
||||
class _SyncASGIBridge(httpx.BaseTransport):
|
||||
def __init__(self, app) -> None:
|
||||
self._async = httpx.ASGITransport(app=app)
|
||||
|
||||
def handle_request(self, request: httpx.Request) -> httpx.Response: # type: ignore[override]
|
||||
async def _drain() -> tuple[int, httpx.Headers, bytes]:
|
||||
async_resp = await self._async.handle_async_request(request)
|
||||
body = await async_resp.aread()
|
||||
await async_resp.aclose()
|
||||
return async_resp.status_code, async_resp.headers, body
|
||||
|
||||
status, headers, body = asyncio.run(_drain())
|
||||
# Wrap the bytes in a fresh sync Response so httpx.Client's
|
||||
# `isinstance(response.stream, SyncByteStream)` assertion holds.
|
||||
return httpx.Response(
|
||||
status_code=status,
|
||||
headers=headers,
|
||||
content=body,
|
||||
request=request,
|
||||
)
|
||||
|
||||
|
||||
# ============================================================================
|
||||
# 1. HMAC headers + signature verify
|
||||
# ============================================================================
|
||||
|
||||
|
||||
async def test_signed_headers_present_and_verify() -> None:
|
||||
captured: list[httpx.Request] = []
|
||||
|
||||
def handler(request: httpx.Request) -> httpx.Response:
|
||||
captured.append(request)
|
||||
# Echo a minimal valid payload for whichever endpoint was hit.
|
||||
return httpx.Response(
|
||||
200,
|
||||
json={
|
||||
"full_name": "octo/widget",
|
||||
"default_branch": "main",
|
||||
"clone_url": "https://example/octo/widget.git",
|
||||
"private": False,
|
||||
},
|
||||
)
|
||||
|
||||
client = GitHubProxyClient(
|
||||
base_url="http://proxy.test",
|
||||
hmac_key=_HMAC,
|
||||
transport=httpx.MockTransport(handler),
|
||||
)
|
||||
info = await client.get_repo("octo/widget")
|
||||
assert isinstance(info, RepoInfo)
|
||||
assert len(captured) == 1
|
||||
req = captured[0]
|
||||
ts = req.headers.get(HEADER_TIMESTAMP)
|
||||
sig = req.headers.get(HEADER_SIGNATURE)
|
||||
assert ts is not None and sig is not None
|
||||
raw_query = req.url.query.decode("ascii")
|
||||
target = f"{req.url.path}?{raw_query}" if raw_query else req.url.path
|
||||
result = verify(
|
||||
method=req.method,
|
||||
path=target,
|
||||
body=req.content or b"",
|
||||
timestamp=ts,
|
||||
signature=sig,
|
||||
key=_HMAC_BYTES,
|
||||
)
|
||||
assert result.ok, result.reason
|
||||
|
||||
|
||||
# ============================================================================
|
||||
# 2. Round-trip via ASGI against a real proxy app
|
||||
# ============================================================================
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def round_trip_app(proxy_settings: Settings):
|
||||
"""A proxy app whose GitHub-side `app.state.github` answers every GH
|
||||
endpoint the GitHubProxyClient exercises in the round-trip test."""
|
||||
app = create_proxy_app(proxy_settings)
|
||||
app.state.settings = proxy_settings
|
||||
|
||||
def gh(req: httpx.Request) -> httpx.Response:
|
||||
path = req.url.path
|
||||
if path == "/repos/octo/widget":
|
||||
return httpx.Response(
|
||||
200,
|
||||
json={
|
||||
"full_name": "octo/widget",
|
||||
"default_branch": "main",
|
||||
"clone_url": "https://example/octo/widget.git",
|
||||
"private": False,
|
||||
},
|
||||
)
|
||||
if path == "/repos/octo/widget/issues/1" and req.method == "GET":
|
||||
return httpx.Response(
|
||||
200,
|
||||
json={
|
||||
"number": 1,
|
||||
"title": "T",
|
||||
"body": "B",
|
||||
"state": "open",
|
||||
"user": {"login": "alice"},
|
||||
"labels": [{"name": "bug"}],
|
||||
},
|
||||
)
|
||||
if path == "/repos/octo/widget/issues" and req.method == "GET":
|
||||
return httpx.Response(
|
||||
200,
|
||||
json=[
|
||||
{
|
||||
"number": 1,
|
||||
"title": "first",
|
||||
"state": "open",
|
||||
"user": {"login": "alice"},
|
||||
"labels": [],
|
||||
"comments": 0,
|
||||
"updated_at": "2026-01-01T00:00:00Z",
|
||||
"created_at": "2026-01-01T00:00:00Z",
|
||||
"html_url": "https://example/1",
|
||||
}
|
||||
],
|
||||
)
|
||||
if path == "/repos/octo/widget/issues/1/comments" and req.method == "GET":
|
||||
return httpx.Response(
|
||||
200,
|
||||
json=[
|
||||
{"id": 7, "user": {"login": "u"}, "body": "hi", "created_at": "2026-01-01T00:00:00Z"},
|
||||
],
|
||||
)
|
||||
if path == "/repos/octo/widget/issues/1/comments" and req.method == "POST":
|
||||
return httpx.Response(
|
||||
201,
|
||||
json={"id": 11, "user": {"login": "bot"}, "body": "posted", "created_at": "2026-01-01T00:00:00Z"},
|
||||
)
|
||||
if path == "/repos/octo/widget/pulls/2/comments":
|
||||
return httpx.Response(
|
||||
200,
|
||||
json=[
|
||||
{
|
||||
"id": 9,
|
||||
"user": {"login": "rev"},
|
||||
"body": "nit",
|
||||
"path": "a.py",
|
||||
"line": 5,
|
||||
"created_at": "2026-01-01T00:00:00Z",
|
||||
}
|
||||
],
|
||||
)
|
||||
if path == "/repos/octo/widget/pulls/2/reviews":
|
||||
return httpx.Response(
|
||||
200,
|
||||
json=[
|
||||
{
|
||||
"id": 12,
|
||||
"user": {"login": "rev"},
|
||||
"body": "approved",
|
||||
"state": "APPROVED",
|
||||
"submitted_at": "2026-01-01T00:00:00Z",
|
||||
}
|
||||
],
|
||||
)
|
||||
if path == "/user":
|
||||
return httpx.Response(200, json={"login": "robomp-bot"})
|
||||
if path == "/repos/octo/widget/pulls/4" and req.method == "GET":
|
||||
return httpx.Response(
|
||||
200,
|
||||
json={
|
||||
"number": 4,
|
||||
"html_url": "https://example/4",
|
||||
"head": {"ref": "feat", "repo": {"full_name": "octo/widget"}},
|
||||
"base": {"ref": "main"},
|
||||
"state": "open",
|
||||
"user": {"login": "robomp-bot"},
|
||||
},
|
||||
)
|
||||
if path == "/repos/octo/widget/pulls" and req.method == "POST":
|
||||
return httpx.Response(
|
||||
201,
|
||||
json={
|
||||
"number": 4,
|
||||
"html_url": "https://example/4",
|
||||
"head": {"ref": "feat"},
|
||||
"base": {"ref": "main"},
|
||||
"state": "open",
|
||||
},
|
||||
)
|
||||
if path == "/repos/octo/widget/pulls/4/requested_reviewers":
|
||||
return httpx.Response(201, json={})
|
||||
if path == "/repos/octo/widget/issues/1/labels":
|
||||
return httpx.Response(200, json=[{"name": "triage"}])
|
||||
if path == "/repos/octo/widget/issues/1/assignees":
|
||||
return httpx.Response(201, json={})
|
||||
return httpx.Response(404, json={"message": f"unrouted {req.method} {path}"})
|
||||
|
||||
_attach_gh(app, gh)
|
||||
return app
|
||||
|
||||
|
||||
async def test_round_trip_all_endpoints(round_trip_app) -> None:
|
||||
client = GitHubProxyClient(
|
||||
base_url="http://proxy.test",
|
||||
hmac_key=_HMAC,
|
||||
transport=httpx.ASGITransport(app=round_trip_app),
|
||||
)
|
||||
repo = await client.get_repo("octo/widget")
|
||||
assert isinstance(repo, RepoInfo)
|
||||
assert repo.full_name == "octo/widget"
|
||||
|
||||
issue = await client.get_issue("octo/widget", 1)
|
||||
assert isinstance(issue, IssueInfo)
|
||||
assert issue.labels == ("bug",)
|
||||
|
||||
issues = await client.list_issues("octo/widget")
|
||||
assert len(issues) == 1 and isinstance(issues[0], IssueSummary)
|
||||
|
||||
comments = await client.list_comments("octo/widget", 1)
|
||||
assert len(comments) == 1 and isinstance(comments[0], CommentInfo)
|
||||
|
||||
rcs = await client.list_review_comments("octo/widget", 2)
|
||||
assert len(rcs) == 1 and isinstance(rcs[0], ReviewCommentInfo)
|
||||
assert rcs[0].line == 5
|
||||
|
||||
prs = await client.list_pr_reviews("octo/widget", 2)
|
||||
assert len(prs) == 1 and isinstance(prs[0], PullRequestReviewInfo)
|
||||
|
||||
assert await client.get_authenticated_login() == "robomp-bot"
|
||||
|
||||
existing_pr = await client.get_pull_request("octo/widget", 4)
|
||||
assert isinstance(existing_pr, PullRequestInfo)
|
||||
assert existing_pr.head_ref == "feat"
|
||||
assert existing_pr.author == "robomp-bot"
|
||||
|
||||
posted = await client.post_comment("octo/widget", 1, "hi")
|
||||
assert isinstance(posted, CommentInfo)
|
||||
assert posted.id == 11
|
||||
|
||||
pr = await client.open_pull_request(repo="octo/widget", head="feat", base="main", title="t", body="b")
|
||||
assert isinstance(pr, PullRequestInfo)
|
||||
assert pr.number == 4
|
||||
|
||||
# request_reviewers returns None on success.
|
||||
assert await client.request_reviewers(repo="octo/widget", pr_number=4, reviewers=["alice"]) is None
|
||||
|
||||
labels = await client.add_issue_labels("octo/widget", 1, ["triage"])
|
||||
assert labels == ("triage",)
|
||||
|
||||
assert await client.add_assignees("octo/widget", 1, ["alice"]) is None
|
||||
|
||||
|
||||
async def test_list_comment_reactions_round_trip(proxy_settings: Settings) -> None:
|
||||
app = create_proxy_app(proxy_settings)
|
||||
app.state.settings = proxy_settings
|
||||
|
||||
def gh(req: httpx.Request) -> httpx.Response:
|
||||
if req.url.path == "/repos/octo/widget/issues/comments/999/reactions":
|
||||
assert req.url.params.get("content") == "-1"
|
||||
return httpx.Response(
|
||||
200,
|
||||
json=[
|
||||
{"content": "-1", "user": {"login": "alice", "type": "User"}},
|
||||
],
|
||||
)
|
||||
return httpx.Response(404, json={"message": "unrouted"})
|
||||
|
||||
_attach_gh(app, gh)
|
||||
client = GitHubProxyClient(
|
||||
base_url="http://proxy.test",
|
||||
hmac_key=_HMAC,
|
||||
transport=httpx.ASGITransport(app=app),
|
||||
)
|
||||
reactions = await client.list_comment_reactions("octo/widget", 999)
|
||||
assert reactions == (ReactionInfo(content="-1", user_login="alice", user_type="User"),)
|
||||
|
||||
|
||||
async def test_close_issue_round_trip(proxy_settings: Settings) -> None:
|
||||
captured: dict[str, object] = {}
|
||||
app = create_proxy_app(proxy_settings)
|
||||
app.state.settings = proxy_settings
|
||||
|
||||
def gh(req: httpx.Request) -> httpx.Response:
|
||||
if req.url.path == "/repos/octo/widget/issues/7" and req.method == "PATCH":
|
||||
captured["body"] = json.loads(req.content)
|
||||
return httpx.Response(200, json={})
|
||||
return httpx.Response(404, json={"message": "unrouted"})
|
||||
|
||||
_attach_gh(app, gh)
|
||||
client = GitHubProxyClient(
|
||||
base_url="http://proxy.test",
|
||||
hmac_key=_HMAC,
|
||||
transport=httpx.ASGITransport(app=app),
|
||||
)
|
||||
assert await client.close_issue("octo/widget", 7) is None
|
||||
assert captured["body"] == {"state": "closed", "state_reason": "completed"}
|
||||
|
||||
|
||||
# ============================================================================
|
||||
# 3. Error decode
|
||||
# ============================================================================
|
||||
|
||||
|
||||
async def test_error_decode_github_422() -> None:
|
||||
def handler(_: httpx.Request) -> httpx.Response:
|
||||
return httpx.Response(
|
||||
422,
|
||||
json={"error": {"kind": "github", "status": 422, "message": "x"}},
|
||||
)
|
||||
|
||||
client = GitHubProxyClient(
|
||||
base_url="http://proxy.test",
|
||||
hmac_key=_HMAC,
|
||||
transport=httpx.MockTransport(handler),
|
||||
)
|
||||
with pytest.raises(GitHubError) as exc:
|
||||
await client.post_comment("octo/widget", 1, "hi")
|
||||
assert exc.value.status == 422
|
||||
assert exc.value.message == "x"
|
||||
|
||||
|
||||
# ============================================================================
|
||||
# 4 + 5. ProxyGitTransport push (happy + HEAD drift)
|
||||
# ============================================================================
|
||||
|
||||
|
||||
def test_proxy_git_transport_push_happy(proxy_settings: Settings, upstream_repo: Path) -> None:
|
||||
branch = "farm/abc/feat"
|
||||
_, head = _stage_workspace(proxy_settings, upstream_repo, "octo/widget", 1, branch)
|
||||
app = create_proxy_app(proxy_settings)
|
||||
app.state.settings = proxy_settings
|
||||
_attach_gh(app, lambda _: httpx.Response(500, json={"message": "should not be hit"}))
|
||||
|
||||
transport = ProxyGitTransport(
|
||||
base_url="http://proxy.test",
|
||||
hmac_key=_HMAC,
|
||||
transport=_SyncASGIBridge(app),
|
||||
)
|
||||
result = transport.push_branch(
|
||||
repo="octo/widget",
|
||||
workspace_key=workspace_key("octo/widget", 1),
|
||||
repo_dir=Path(proxy_settings.workspace_root) / workspace_key("octo/widget", 1) / "repo",
|
||||
branch=branch,
|
||||
expected_head=head,
|
||||
)
|
||||
assert result.head == head
|
||||
assert result.branch == branch
|
||||
assert _bare_has_branch(upstream_repo, branch)
|
||||
|
||||
|
||||
def test_proxy_git_transport_push_head_drift(proxy_settings: Settings, upstream_repo: Path) -> None:
|
||||
branch = "farm/abc/drift"
|
||||
_, _ = _stage_workspace(proxy_settings, upstream_repo, "octo/widget", 1, branch)
|
||||
app = create_proxy_app(proxy_settings)
|
||||
app.state.settings = proxy_settings
|
||||
_attach_gh(app, lambda _: httpx.Response(500, json={"message": "should not be hit"}))
|
||||
|
||||
transport = ProxyGitTransport(
|
||||
base_url="http://proxy.test",
|
||||
hmac_key=_HMAC,
|
||||
transport=_SyncASGIBridge(app),
|
||||
)
|
||||
with pytest.raises(HeadDriftError):
|
||||
transport.push_branch(
|
||||
repo="octo/widget",
|
||||
workspace_key=workspace_key("octo/widget", 1),
|
||||
repo_dir=Path(proxy_settings.workspace_root) / workspace_key("octo/widget", 1) / "repo",
|
||||
branch=branch,
|
||||
expected_head="0" * 40,
|
||||
)
|
||||
assert not _bare_has_branch(upstream_repo, branch)
|
||||
|
||||
|
||||
def test_proxy_git_transport_push_slot_uid_body() -> None:
|
||||
captured: list[dict[str, object]] = []
|
||||
|
||||
def handler(request: httpx.Request) -> httpx.Response:
|
||||
captured.append(json.loads(request.content))
|
||||
return httpx.Response(200, json={"head": "abc123", "branch": "farm/abc/feat"})
|
||||
|
||||
transport = ProxyGitTransport(
|
||||
base_url="http://proxy.test",
|
||||
hmac_key=_HMAC,
|
||||
transport=httpx.MockTransport(handler),
|
||||
)
|
||||
transport.push_branch(
|
||||
repo="octo/widget",
|
||||
workspace_key="octo__widget__1",
|
||||
repo_dir=Path("/unused"),
|
||||
branch="farm/abc/feat",
|
||||
expected_head="abc123",
|
||||
slot_uid=2001,
|
||||
)
|
||||
transport.push_branch(
|
||||
repo="octo/widget",
|
||||
workspace_key="octo__widget__1",
|
||||
repo_dir=Path("/unused"),
|
||||
branch="farm/abc/feat",
|
||||
expected_head="abc123",
|
||||
)
|
||||
|
||||
assert captured[0]["slot_uid"] == 2001
|
||||
assert "slot_uid" not in captured[1]
|
||||
|
||||
|
||||
# Sanity: signed POST headers from ProxyGitTransport._post verify cleanly.
|
||||
def test_proxy_git_transport_post_headers_verify() -> None:
|
||||
captured: list[httpx.Request] = []
|
||||
|
||||
def handler(request: httpx.Request) -> httpx.Response:
|
||||
captured.append(request)
|
||||
return httpx.Response(200, json={"pool_dir": "/tmp/x"})
|
||||
|
||||
transport = ProxyGitTransport(
|
||||
base_url="http://proxy.test",
|
||||
hmac_key=_HMAC,
|
||||
transport=httpx.MockTransport(handler),
|
||||
)
|
||||
transport.clone_pool(
|
||||
repo="octo/widget",
|
||||
clone_url="https://example/widget.git",
|
||||
default_branch="main",
|
||||
target=Path("/tmp/unused"),
|
||||
)
|
||||
assert len(captured) == 1
|
||||
req = captured[0]
|
||||
ts = req.headers.get(HEADER_TIMESTAMP)
|
||||
sig = req.headers.get(HEADER_SIGNATURE)
|
||||
assert ts and sig
|
||||
result = verify(
|
||||
method="POST",
|
||||
path="/gh/v1/git/clone",
|
||||
body=req.content or b"",
|
||||
timestamp=ts,
|
||||
signature=sig,
|
||||
key=_HMAC_BYTES,
|
||||
)
|
||||
assert result.ok, result.reason
|
||||
assert json.loads(req.content)["repo"] == "octo/widget"
|
||||
@@ -0,0 +1,993 @@
|
||||
"""HMAC + endpoint coverage for the gh-proxy FastAPI app."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import os
|
||||
import platform
|
||||
import subprocess
|
||||
import time
|
||||
from collections.abc import Callable
|
||||
from pathlib import Path
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
from pydantic import SecretStr
|
||||
|
||||
from robomp.config import Settings
|
||||
from robomp.github_client import GitHubClient
|
||||
from robomp.proxy.server import create_proxy_app
|
||||
from robomp.proxy_hmac import HEADER_SIGNATURE, HEADER_TIMESTAMP, sign
|
||||
from robomp.sandbox import workspace_key
|
||||
|
||||
_HMAC = "test-hmac-key-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa"
|
||||
_TOKEN = "ghp_test_token_value"
|
||||
|
||||
|
||||
# ---------- shared fixtures ----------
|
||||
|
||||
|
||||
def _build_settings(tmp_path: Path) -> Settings:
|
||||
"""Construct a Settings object for the proxy side without going through
|
||||
the orchestrator-mode mutual-exclusion validator (the proxy reads token +
|
||||
hmac key directly; the validator is geared at orchestrator deployments)."""
|
||||
cfg = Settings.model_construct(
|
||||
github_token=SecretStr(_TOKEN),
|
||||
github_webhook_secret=SecretStr("webhook-secret"),
|
||||
bot_login="robomp-bot",
|
||||
git_author_email="robomp-bot@example.invalid",
|
||||
repo_allowlist_raw="octo/widget",
|
||||
gh_proxy_url=None,
|
||||
gh_proxy_hmac_key=SecretStr(_HMAC),
|
||||
gh_proxy_bind_host="0.0.0.0",
|
||||
gh_proxy_bind_port=8081,
|
||||
workspace_root=tmp_path / "workspaces",
|
||||
sqlite_path=tmp_path / "robomp.sqlite",
|
||||
log_dir=tmp_path / "logs",
|
||||
)
|
||||
cfg.ensure_paths()
|
||||
return cfg
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def proxy_settings(tmp_path: Path) -> Settings:
|
||||
return _build_settings(tmp_path)
|
||||
|
||||
|
||||
def _git(args: list[str], cwd: Path) -> subprocess.CompletedProcess[str]:
|
||||
return subprocess.run(
|
||||
["git", *args],
|
||||
cwd=str(cwd),
|
||||
check=True,
|
||||
capture_output=True,
|
||||
text=True,
|
||||
env=os.environ
|
||||
| {
|
||||
"GIT_AUTHOR_NAME": "t",
|
||||
"GIT_AUTHOR_EMAIL": "t@t",
|
||||
"GIT_COMMITTER_NAME": "t",
|
||||
"GIT_COMMITTER_EMAIL": "t@t",
|
||||
},
|
||||
)
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def upstream_repo(tmp_path: Path) -> Path:
|
||||
"""Bare local repo with one commit on `main`."""
|
||||
repo = tmp_path / "upstream.git"
|
||||
repo.mkdir()
|
||||
_git(["init", "--initial-branch=main", "--bare", str(repo)], tmp_path)
|
||||
seed = tmp_path / "seed"
|
||||
seed.mkdir()
|
||||
_git(["init", "--initial-branch=main", str(seed)], tmp_path)
|
||||
(seed / "README.md").write_text("hello\n", encoding="utf-8")
|
||||
_git(["-C", str(seed), "add", "."], tmp_path)
|
||||
_git(["-C", str(seed), "commit", "-m", "init"], tmp_path)
|
||||
_git(["-C", str(seed), "remote", "add", "origin", str(repo)], tmp_path)
|
||||
_git(["-C", str(seed), "push", "origin", "main"], tmp_path)
|
||||
return repo
|
||||
|
||||
|
||||
def _stage_workspace(cfg: Settings, upstream: Path, repo: str, number: int, branch: str) -> tuple[Path, str]:
|
||||
"""Pre-stage a workspace clone with one new commit on `branch`."""
|
||||
ws_dir = Path(cfg.workspace_root) / workspace_key(repo, number)
|
||||
ws_dir.mkdir(parents=True, exist_ok=True)
|
||||
repo_dir = ws_dir / "repo"
|
||||
_git(["clone", str(upstream), str(repo_dir)], ws_dir)
|
||||
_git(["-C", str(repo_dir), "config", "user.email", "t@t"], ws_dir)
|
||||
_git(["-C", str(repo_dir), "config", "user.name", "t"], ws_dir)
|
||||
_git(["-C", str(repo_dir), "checkout", "-b", branch], ws_dir)
|
||||
(repo_dir / "x.txt").write_text("x", encoding="utf-8")
|
||||
_git(["-C", str(repo_dir), "add", "."], ws_dir)
|
||||
_git(["-C", str(repo_dir), "commit", "-m", "x"], ws_dir)
|
||||
proc = _git(["-C", str(repo_dir), "rev-parse", "HEAD"], ws_dir)
|
||||
return repo_dir, proc.stdout.strip()
|
||||
|
||||
|
||||
def _bare_has_branch(bare: Path, branch: str) -> bool:
|
||||
proc = subprocess.run(
|
||||
["git", "-C", str(bare), "branch", "--list", branch],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=False,
|
||||
)
|
||||
return bool(proc.stdout.strip())
|
||||
|
||||
|
||||
# ---------- HMAC + signed request helpers ----------
|
||||
|
||||
|
||||
def _signed(
|
||||
method: str,
|
||||
path: str,
|
||||
body: bytes = b"",
|
||||
*,
|
||||
params: dict[str, object] | None = None,
|
||||
ts: str | None = None,
|
||||
key: bytes | None = None,
|
||||
) -> dict[str, str]:
|
||||
"""Build signed headers.
|
||||
|
||||
When `params` is supplied, the canonical signing target becomes
|
||||
`path?<query>` (matching the verifier's request-target reconstruction),
|
||||
so signed requests with query strings stay verifiable AND mutating any
|
||||
query parameter post-sign produces a 401.
|
||||
"""
|
||||
if params:
|
||||
url = httpx.URL(path, params=params)
|
||||
query = url.query.decode("ascii") if url.query else ""
|
||||
target = f"{path}?{query}" if query else path
|
||||
else:
|
||||
target = path
|
||||
timestamp, sig = sign(method=method, path=target, body=body, key=key or _HMAC.encode(), timestamp=ts)
|
||||
return {HEADER_TIMESTAMP: timestamp, HEADER_SIGNATURE: sig}
|
||||
|
||||
|
||||
def _build_app(cfg: Settings, gh_handler: Callable[[httpx.Request], httpx.Response] | None = None):
|
||||
app = create_proxy_app(cfg)
|
||||
transport = httpx.MockTransport(gh_handler) if gh_handler is not None else None
|
||||
app.state.github = GitHubClient(_TOKEN, transport=transport)
|
||||
app.state.settings = cfg
|
||||
return app
|
||||
|
||||
|
||||
async def _async_client(app) -> httpx.AsyncClient:
|
||||
return httpx.AsyncClient(
|
||||
transport=httpx.ASGITransport(app=app),
|
||||
base_url="http://proxy.test",
|
||||
)
|
||||
|
||||
|
||||
def test_read_origin_url_uses_safe_directory_and_slot_identity(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
from robomp.proxy import server as proxy_server
|
||||
|
||||
captured: dict[str, object] = {}
|
||||
repo_dir = tmp_path / "repo"
|
||||
|
||||
def fake_run(cmd: list[str], **kwargs: object) -> subprocess.CompletedProcess[str]:
|
||||
captured["cmd"] = cmd
|
||||
captured.update(kwargs)
|
||||
return subprocess.CompletedProcess(cmd, 0, "https://github.com/octo/widget.git\n", "")
|
||||
|
||||
monkeypatch.setattr("robomp.proxy.server.subprocess.run", fake_run)
|
||||
monkeypatch.setattr(
|
||||
"robomp.proxy.server._slot_subprocess_kwargs",
|
||||
lambda uid: {"user": uid, "group": uid, "extra_groups": [2000], "umask": 0o002},
|
||||
)
|
||||
|
||||
assert proxy_server._read_origin_url(repo_dir, slot_uid=2001) == "https://github.com/octo/widget.git"
|
||||
|
||||
env = captured["env"]
|
||||
assert isinstance(env, dict)
|
||||
assert env["GIT_CONFIG_COUNT"] == "1"
|
||||
assert env["GIT_CONFIG_KEY_0"] == "safe.directory"
|
||||
assert env["GIT_CONFIG_VALUE_0"] == str(repo_dir)
|
||||
assert captured["user"] == 2001
|
||||
assert captured["group"] == 2001
|
||||
assert captured["extra_groups"] == [2000]
|
||||
assert captured["umask"] == 0o002
|
||||
|
||||
|
||||
# ============================================================================
|
||||
# HMAC behavior
|
||||
# ============================================================================
|
||||
|
||||
|
||||
async def test_hmac_accept_post_comment_round_trip(proxy_settings: Settings) -> None:
|
||||
captured: dict[str, httpx.Request] = {}
|
||||
|
||||
def gh(req: httpx.Request) -> httpx.Response:
|
||||
captured["req"] = req
|
||||
return httpx.Response(
|
||||
201,
|
||||
json={
|
||||
"id": 42,
|
||||
"user": {"login": "robomp-bot"},
|
||||
"body": "hello",
|
||||
"created_at": "2026-01-01T00:00:00Z",
|
||||
},
|
||||
)
|
||||
|
||||
app = _build_app(proxy_settings, gh)
|
||||
body = b'{"repo":"octo/widget","number":1,"body":"hello"}'
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.post(
|
||||
"/gh/v1/post_comment",
|
||||
content=body,
|
||||
headers={**_signed("POST", "/gh/v1/post_comment", body), "Content-Type": "application/json"},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert resp.json() == {"id": 42, "author": "robomp-bot", "body": "hello", "created_at": "2026-01-01T00:00:00Z"}
|
||||
assert captured["req"].url.path == "/repos/octo/widget/issues/1/comments"
|
||||
|
||||
|
||||
async def test_hmac_reject_missing_headers(proxy_settings: Settings) -> None:
|
||||
app = _build_app(proxy_settings, lambda _: httpx.Response(200, json={}))
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.get("/gh/v1/repo", params={"repo": "octo/widget"})
|
||||
assert resp.status_code == 401
|
||||
|
||||
|
||||
async def test_hmac_reject_bad_signature(proxy_settings: Settings) -> None:
|
||||
app = _build_app(proxy_settings, lambda _: httpx.Response(200, json={}))
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.get(
|
||||
"/gh/v1/repo",
|
||||
params={"repo": "octo/widget"},
|
||||
headers={HEADER_TIMESTAMP: str(int(time.time())), HEADER_SIGNATURE: "0" * 64},
|
||||
)
|
||||
assert resp.status_code == 401
|
||||
|
||||
|
||||
async def test_hmac_reject_stale_timestamp(proxy_settings: Settings) -> None:
|
||||
app = _build_app(proxy_settings, lambda _: httpx.Response(200, json={}))
|
||||
stale = str(int(time.time()) - 120)
|
||||
headers = _signed("GET", "/gh/v1/repo", ts=stale)
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.get("/gh/v1/repo", params={"repo": "octo/widget"}, headers=headers)
|
||||
assert resp.status_code == 401
|
||||
|
||||
|
||||
# ============================================================================
|
||||
# GET endpoints
|
||||
# ============================================================================
|
||||
|
||||
|
||||
async def test_get_repo(proxy_settings: Settings) -> None:
|
||||
def gh(req: httpx.Request) -> httpx.Response:
|
||||
assert req.url.path == "/repos/octo/widget"
|
||||
return httpx.Response(
|
||||
200,
|
||||
json={
|
||||
"full_name": "octo/widget",
|
||||
"default_branch": "main",
|
||||
"clone_url": "https://github.com/octo/widget.git",
|
||||
"private": False,
|
||||
},
|
||||
)
|
||||
|
||||
app = _build_app(proxy_settings, gh)
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.get(
|
||||
"/gh/v1/repo",
|
||||
params={"repo": "octo/widget"},
|
||||
headers=_signed("GET", "/gh/v1/repo", params={"repo": "octo/widget"}),
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert resp.json() == {
|
||||
"full_name": "octo/widget",
|
||||
"default_branch": "main",
|
||||
"clone_url": "https://github.com/octo/widget.git",
|
||||
"private": False,
|
||||
}
|
||||
|
||||
|
||||
async def test_get_issue(proxy_settings: Settings) -> None:
|
||||
def gh(req: httpx.Request) -> httpx.Response:
|
||||
assert req.url.path == "/repos/octo/widget/issues/1"
|
||||
return httpx.Response(
|
||||
200,
|
||||
json={
|
||||
"number": 1,
|
||||
"title": "T",
|
||||
"body": "B",
|
||||
"state": "open",
|
||||
"user": {"login": "alice"},
|
||||
"labels": [{"name": "bug"}],
|
||||
},
|
||||
)
|
||||
|
||||
app = _build_app(proxy_settings, gh)
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.get(
|
||||
"/gh/v1/issue",
|
||||
params={"repo": "octo/widget", "number": 1},
|
||||
headers=_signed("GET", "/gh/v1/issue", params={"repo": "octo/widget", "number": 1}),
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
payload = resp.json()
|
||||
assert payload["repo"] == "octo/widget"
|
||||
assert payload["number"] == 1
|
||||
assert payload["labels"] == ["bug"]
|
||||
assert payload["is_pull_request"] is False
|
||||
|
||||
|
||||
async def test_list_issues(proxy_settings: Settings) -> None:
|
||||
def gh(req: httpx.Request) -> httpx.Response:
|
||||
assert req.url.path == "/repos/octo/widget/issues"
|
||||
return httpx.Response(
|
||||
200,
|
||||
json=[
|
||||
{
|
||||
"number": 1,
|
||||
"title": "first",
|
||||
"state": "open",
|
||||
"user": {"login": "alice"},
|
||||
"labels": [{"name": "bug"}],
|
||||
"comments": 0,
|
||||
"updated_at": "2026-01-01T00:00:00Z",
|
||||
"created_at": "2026-01-01T00:00:00Z",
|
||||
"html_url": "https://example/1",
|
||||
},
|
||||
# A PR — must be filtered out.
|
||||
{
|
||||
"number": 2,
|
||||
"title": "pr",
|
||||
"pull_request": {"url": "x"},
|
||||
"user": {"login": "alice"},
|
||||
},
|
||||
],
|
||||
)
|
||||
|
||||
app = _build_app(proxy_settings, gh)
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.get(
|
||||
"/gh/v1/issues",
|
||||
params={"repo": "octo/widget"},
|
||||
headers=_signed("GET", "/gh/v1/issues", params={"repo": "octo/widget"}),
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
items = resp.json()["items"]
|
||||
assert len(items) == 1
|
||||
assert items[0]["number"] == 1
|
||||
|
||||
|
||||
async def test_list_comments(proxy_settings: Settings) -> None:
|
||||
def gh(req: httpx.Request) -> httpx.Response:
|
||||
assert req.url.path == "/repos/octo/widget/issues/1/comments"
|
||||
return httpx.Response(
|
||||
200,
|
||||
json=[
|
||||
{"id": 1, "user": {"login": "u"}, "body": "hi", "created_at": "2026-01-01T00:00:00Z"},
|
||||
],
|
||||
)
|
||||
|
||||
app = _build_app(proxy_settings, gh)
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.get(
|
||||
"/gh/v1/comments",
|
||||
params={"repo": "octo/widget", "number": 1},
|
||||
headers=_signed("GET", "/gh/v1/comments", params={"repo": "octo/widget", "number": 1}),
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert resp.json() == {
|
||||
"items": [{"id": 1, "author": "u", "body": "hi", "created_at": "2026-01-01T00:00:00Z"}],
|
||||
}
|
||||
|
||||
|
||||
async def test_list_review_comments(proxy_settings: Settings) -> None:
|
||||
def gh(req: httpx.Request) -> httpx.Response:
|
||||
assert req.url.path == "/repos/octo/widget/pulls/1/comments"
|
||||
return httpx.Response(
|
||||
200,
|
||||
json=[
|
||||
{
|
||||
"id": 9,
|
||||
"user": {"login": "rev"},
|
||||
"body": "nit",
|
||||
"path": "a.py",
|
||||
"line": 5,
|
||||
"created_at": "2026-01-01T00:00:00Z",
|
||||
}
|
||||
],
|
||||
)
|
||||
|
||||
app = _build_app(proxy_settings, gh)
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.get(
|
||||
"/gh/v1/review_comments",
|
||||
params={"repo": "octo/widget", "pr_number": 1},
|
||||
headers=_signed("GET", "/gh/v1/review_comments", params={"repo": "octo/widget", "pr_number": 1}),
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
items = resp.json()["items"]
|
||||
assert items[0]["path"] == "a.py"
|
||||
assert items[0]["line"] == 5
|
||||
|
||||
|
||||
async def test_list_pr_reviews(proxy_settings: Settings) -> None:
|
||||
def gh(req: httpx.Request) -> httpx.Response:
|
||||
assert req.url.path == "/repos/octo/widget/pulls/1/reviews"
|
||||
return httpx.Response(
|
||||
200,
|
||||
json=[
|
||||
{
|
||||
"id": 11,
|
||||
"user": {"login": "rev"},
|
||||
"body": "looks good",
|
||||
"state": "APPROVED",
|
||||
"submitted_at": "2026-01-01T00:00:00Z",
|
||||
},
|
||||
# Empty body — must be filtered out by GitHubClient.
|
||||
{"id": 12, "user": {"login": "rev"}, "body": " ", "state": "COMMENTED"},
|
||||
],
|
||||
)
|
||||
|
||||
app = _build_app(proxy_settings, gh)
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.get(
|
||||
"/gh/v1/pr_reviews",
|
||||
params={"repo": "octo/widget", "pr_number": 1},
|
||||
headers=_signed("GET", "/gh/v1/pr_reviews", params={"repo": "octo/widget", "pr_number": 1}),
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
items = resp.json()["items"]
|
||||
assert len(items) == 1
|
||||
assert items[0]["state"] == "APPROVED"
|
||||
|
||||
|
||||
async def test_authenticated_login(proxy_settings: Settings) -> None:
|
||||
def gh(req: httpx.Request) -> httpx.Response:
|
||||
assert req.url.path == "/user"
|
||||
return httpx.Response(200, json={"login": "robomp-bot"})
|
||||
|
||||
app = _build_app(proxy_settings, gh)
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.get(
|
||||
"/gh/v1/authenticated_login",
|
||||
headers=_signed("GET", "/gh/v1/authenticated_login"),
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert resp.json() == {"login": "robomp-bot"}
|
||||
|
||||
|
||||
# ============================================================================
|
||||
# POST endpoints
|
||||
# ============================================================================
|
||||
|
||||
|
||||
async def test_post_comment_forwards_body(proxy_settings: Settings) -> None:
|
||||
captured: dict[str, httpx.Request] = {}
|
||||
|
||||
def gh(req: httpx.Request) -> httpx.Response:
|
||||
captured["req"] = req
|
||||
return httpx.Response(
|
||||
201,
|
||||
json={"id": 7, "user": {"login": "b"}, "body": "hi", "created_at": "2026-01-01T00:00:00Z"},
|
||||
)
|
||||
|
||||
app = _build_app(proxy_settings, gh)
|
||||
body = b'{"repo":"octo/widget","number":1,"body":"hi"}'
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.post(
|
||||
"/gh/v1/post_comment",
|
||||
content=body,
|
||||
headers={**_signed("POST", "/gh/v1/post_comment", body), "Content-Type": "application/json"},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
req = captured["req"]
|
||||
assert req.method == "POST"
|
||||
assert req.url.path == "/repos/octo/widget/issues/1/comments"
|
||||
import json
|
||||
|
||||
assert json.loads(req.content) == {"body": "hi"}
|
||||
|
||||
|
||||
async def test_add_issue_labels(proxy_settings: Settings) -> None:
|
||||
captured: dict[str, httpx.Request] = {}
|
||||
|
||||
def gh(req: httpx.Request) -> httpx.Response:
|
||||
captured["req"] = req
|
||||
return httpx.Response(200, json=[{"name": "triage"}, {"name": "bug"}])
|
||||
|
||||
app = _build_app(proxy_settings, gh)
|
||||
body = b'{"repo":"octo/widget","number":1,"labels":["triage","bug"]}'
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.post(
|
||||
"/gh/v1/add_issue_labels",
|
||||
content=body,
|
||||
headers={**_signed("POST", "/gh/v1/add_issue_labels", body), "Content-Type": "application/json"},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert resp.json() == {"labels": ["triage", "bug"]}
|
||||
assert captured["req"].url.path == "/repos/octo/widget/issues/1/labels"
|
||||
import json
|
||||
|
||||
assert json.loads(captured["req"].content) == {"labels": ["triage", "bug"]}
|
||||
|
||||
|
||||
async def test_add_assignees(proxy_settings: Settings) -> None:
|
||||
captured: dict[str, httpx.Request] = {}
|
||||
|
||||
def gh(req: httpx.Request) -> httpx.Response:
|
||||
captured["req"] = req
|
||||
return httpx.Response(201, json={})
|
||||
|
||||
app = _build_app(proxy_settings, gh)
|
||||
body = b'{"repo":"octo/widget","number":1,"assignees":["alice"]}'
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.post(
|
||||
"/gh/v1/add_assignees",
|
||||
content=body,
|
||||
headers={**_signed("POST", "/gh/v1/add_assignees", body), "Content-Type": "application/json"},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert resp.json() == {"ok": True}
|
||||
assert captured["req"].url.path == "/repos/octo/widget/issues/1/assignees"
|
||||
import json
|
||||
|
||||
assert json.loads(captured["req"].content) == {"assignees": ["alice"]}
|
||||
|
||||
|
||||
async def test_comment_reactions(proxy_settings: Settings) -> None:
|
||||
captured: dict[str, httpx.Request] = {}
|
||||
|
||||
def gh(req: httpx.Request) -> httpx.Response:
|
||||
captured["req"] = req
|
||||
return httpx.Response(
|
||||
200,
|
||||
json=[
|
||||
{"content": "-1", "user": {"login": "alice", "type": "User"}},
|
||||
],
|
||||
)
|
||||
|
||||
app = _build_app(proxy_settings, gh)
|
||||
target = "/gh/v1/comment_reactions?repo=octo%2Fwidget&comment_id=999"
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.get(target, headers=_signed("GET", target))
|
||||
assert resp.status_code == 200
|
||||
assert resp.json() == {
|
||||
"items": [{"content": "-1", "user_login": "alice", "user_type": "User"}],
|
||||
}
|
||||
req = captured["req"]
|
||||
assert req.method == "GET"
|
||||
assert req.url.path == "/repos/octo/widget/issues/comments/999/reactions"
|
||||
assert req.url.params.get("content") == "-1"
|
||||
|
||||
|
||||
async def test_close_issue(proxy_settings: Settings) -> None:
|
||||
captured: dict[str, httpx.Request] = {}
|
||||
|
||||
def gh(req: httpx.Request) -> httpx.Response:
|
||||
captured["req"] = req
|
||||
return httpx.Response(200, json={})
|
||||
|
||||
app = _build_app(proxy_settings, gh)
|
||||
body = b'{"repo":"octo/widget","number":7,"reason":"completed"}'
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.post(
|
||||
"/gh/v1/close_issue",
|
||||
content=body,
|
||||
headers={**_signed("POST", "/gh/v1/close_issue", body), "Content-Type": "application/json"},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert resp.json() == {"ok": True}
|
||||
req = captured["req"]
|
||||
assert req.method == "PATCH"
|
||||
assert req.url.path == "/repos/octo/widget/issues/7"
|
||||
import json
|
||||
|
||||
assert json.loads(req.content) == {"state": "closed", "state_reason": "completed"}
|
||||
|
||||
|
||||
async def test_close_issue_defaults_reason_to_completed(proxy_settings: Settings) -> None:
|
||||
captured: dict[str, httpx.Request] = {}
|
||||
|
||||
def gh(req: httpx.Request) -> httpx.Response:
|
||||
captured["req"] = req
|
||||
return httpx.Response(200, json={})
|
||||
|
||||
app = _build_app(proxy_settings, gh)
|
||||
body = b'{"repo":"octo/widget","number":7}'
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.post(
|
||||
"/gh/v1/close_issue",
|
||||
content=body,
|
||||
headers={**_signed("POST", "/gh/v1/close_issue", body), "Content-Type": "application/json"},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
import json
|
||||
|
||||
assert json.loads(captured["req"].content) == {"state": "closed", "state_reason": "completed"}
|
||||
|
||||
|
||||
async def test_open_pull_request(proxy_settings: Settings) -> None:
|
||||
captured: dict[str, httpx.Request] = {}
|
||||
|
||||
def gh(req: httpx.Request) -> httpx.Response:
|
||||
captured["req"] = req
|
||||
return httpx.Response(
|
||||
201,
|
||||
json={
|
||||
"number": 4,
|
||||
"html_url": "https://example/4",
|
||||
"head": {"ref": "feature"},
|
||||
"base": {"ref": "main"},
|
||||
"state": "open",
|
||||
},
|
||||
)
|
||||
|
||||
app = _build_app(proxy_settings, gh)
|
||||
body = (
|
||||
b'{"repo":"octo/widget","head":"feature","base":"main",'
|
||||
b'"title":"t","body":"b","draft":false,"maintainer_can_modify":true}'
|
||||
)
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.post(
|
||||
"/gh/v1/open_pull_request",
|
||||
content=body,
|
||||
headers={**_signed("POST", "/gh/v1/open_pull_request", body), "Content-Type": "application/json"},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert resp.json()["number"] == 4
|
||||
assert captured["req"].url.path == "/repos/octo/widget/pulls"
|
||||
import json
|
||||
|
||||
sent = json.loads(captured["req"].content)
|
||||
assert sent["head"] == "feature"
|
||||
assert sent["base"] == "main"
|
||||
assert sent["title"] == "t"
|
||||
|
||||
|
||||
async def test_request_reviewers(proxy_settings: Settings) -> None:
|
||||
captured: dict[str, httpx.Request] = {}
|
||||
|
||||
def gh(req: httpx.Request) -> httpx.Response:
|
||||
captured["req"] = req
|
||||
return httpx.Response(201, json={})
|
||||
|
||||
app = _build_app(proxy_settings, gh)
|
||||
body = b'{"repo":"octo/widget","pr_number":4,"reviewers":["alice"],"team_reviewers":null}'
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.post(
|
||||
"/gh/v1/request_reviewers",
|
||||
content=body,
|
||||
headers={**_signed("POST", "/gh/v1/request_reviewers", body), "Content-Type": "application/json"},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
assert resp.json() == {"ok": True}
|
||||
assert captured["req"].url.path == "/repos/octo/widget/pulls/4/requested_reviewers"
|
||||
import json
|
||||
|
||||
assert json.loads(captured["req"].content) == {"reviewers": ["alice"]}
|
||||
|
||||
|
||||
# ============================================================================
|
||||
# GitHub error passthrough
|
||||
# ============================================================================
|
||||
|
||||
|
||||
async def test_github_error_passthrough_422(proxy_settings: Settings) -> None:
|
||||
def gh(_: httpx.Request) -> httpx.Response:
|
||||
return httpx.Response(422, json={"message": "validation failed"})
|
||||
|
||||
app = _build_app(proxy_settings, gh)
|
||||
body = b'{"repo":"octo/widget","number":1,"body":"hi"}'
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.post(
|
||||
"/gh/v1/post_comment",
|
||||
content=body,
|
||||
headers={**_signed("POST", "/gh/v1/post_comment", body), "Content-Type": "application/json"},
|
||||
)
|
||||
assert resp.status_code == 422
|
||||
err = resp.json()["error"]
|
||||
assert err["kind"] == "github"
|
||||
assert err["status"] == 422
|
||||
assert err["message"] == "validation failed"
|
||||
|
||||
|
||||
# ============================================================================
|
||||
# git transport endpoints
|
||||
# ============================================================================
|
||||
|
||||
|
||||
async def test_git_clone_creates_pool_dir(proxy_settings: Settings, upstream_repo: Path) -> None:
|
||||
app = _build_app(proxy_settings)
|
||||
body = b'{"repo":"octo/widget","clone_url":"' + str(upstream_repo).encode() + b'","default_branch":"main"}'
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.post(
|
||||
"/gh/v1/git/clone",
|
||||
content=body,
|
||||
headers={**_signed("POST", "/gh/v1/git/clone", body), "Content-Type": "application/json"},
|
||||
)
|
||||
assert resp.status_code == 200
|
||||
pool_dir = Path(resp.json()["pool_dir"])
|
||||
assert pool_dir.is_dir()
|
||||
assert pool_dir == Path(proxy_settings.workspace_root) / "_pool" / "octo__widget"
|
||||
assert (pool_dir / "HEAD").exists() or (pool_dir / ".git" / "HEAD").exists()
|
||||
|
||||
|
||||
async def test_git_fetch_repairs_missing_alternate_and_bad_ref(proxy_settings: Settings, upstream_repo: Path) -> None:
|
||||
pool_dir = Path(proxy_settings.workspace_root) / "_pool" / "octo__widget"
|
||||
pool_dir.parent.mkdir(parents=True, exist_ok=True)
|
||||
_git(["clone", "--filter=blob:none", str(upstream_repo), str(pool_dir)], Path(proxy_settings.workspace_root))
|
||||
|
||||
bad_ref = pool_dir / ".git" / "refs" / "heads" / "farm" / "bad"
|
||||
bad_ref.parent.mkdir(parents=True, exist_ok=True)
|
||||
bad_ref.write_text("0123456789012345678901234567890123456789\n", encoding="ascii")
|
||||
|
||||
alternates = pool_dir / ".git" / "objects" / "info" / "alternates"
|
||||
alternates.write_text(str(Path(proxy_settings.workspace_root) / "missing-objects") + "\n", encoding="utf-8")
|
||||
|
||||
app = _build_app(proxy_settings)
|
||||
body = b'{"repo":"octo/widget"}'
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.post(
|
||||
"/gh/v1/git/fetch",
|
||||
content=body,
|
||||
headers={**_signed("POST", "/gh/v1/git/fetch", body), "Content-Type": "application/json"},
|
||||
)
|
||||
|
||||
assert resp.status_code == 200, resp.text
|
||||
assert Path(resp.json()["pool_dir"]) == pool_dir
|
||||
assert not bad_ref.exists()
|
||||
assert not alternates.exists()
|
||||
|
||||
|
||||
async def test_git_push_happy_path(proxy_settings: Settings, upstream_repo: Path) -> None:
|
||||
branch = "farm/abc/feature"
|
||||
_, head = _stage_workspace(proxy_settings, upstream_repo, "octo/widget", 1, branch)
|
||||
# Rewire origin to the bare upstream so the proxy's push lands there.
|
||||
app = _build_app(proxy_settings)
|
||||
body = (
|
||||
b'{"repo":"octo/widget","workspace_key":"octo__widget__1","branch":"'
|
||||
+ branch.encode()
|
||||
+ b'","expected_head":"'
|
||||
+ head.encode()
|
||||
+ b'"}'
|
||||
)
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.post(
|
||||
"/gh/v1/git/push",
|
||||
content=body,
|
||||
headers={**_signed("POST", "/gh/v1/git/push", body), "Content-Type": "application/json"},
|
||||
)
|
||||
assert resp.status_code == 200, resp.text
|
||||
assert resp.json() == {"head": head, "branch": branch}
|
||||
assert _bare_has_branch(upstream_repo, branch)
|
||||
|
||||
|
||||
async def test_git_push_passes_slot_uid_to_git_push(
|
||||
proxy_settings: Settings, upstream_repo: Path, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
from robomp.git_ops import PushResult
|
||||
|
||||
branch = "farm/abc/slot"
|
||||
repo_dir, head = _stage_workspace(proxy_settings, upstream_repo, "octo/widget", 1, branch)
|
||||
# The push handler reads the origin URL as the slot uid. On Linux+root
|
||||
# the staged workspace is root-owned; hand it to slot 2001 so the
|
||||
# subprocess can stat it. On macOS dev this is a no-op (slot identity
|
||||
# is never activated).
|
||||
if platform.system() == "Linux" and os.geteuid() == 0:
|
||||
for path in [repo_dir.parent, repo_dir, *repo_dir.rglob("*")]:
|
||||
os.chown(path, 2001, 2001, follow_symlinks=False)
|
||||
captured: dict[str, object] = {}
|
||||
|
||||
def fake_git_push(path: Path, **kwargs: object) -> PushResult:
|
||||
captured["path"] = path
|
||||
captured.update(kwargs)
|
||||
return PushResult(head=head, branch=branch)
|
||||
|
||||
monkeypatch.setattr("robomp.proxy.server.git_push", fake_git_push)
|
||||
app = _build_app(proxy_settings)
|
||||
body = (
|
||||
b'{"repo":"octo/widget","workspace_key":"octo__widget__1","branch":"'
|
||||
+ branch.encode()
|
||||
+ b'","expected_head":"'
|
||||
+ head.encode()
|
||||
+ b'","slot_uid":2001}'
|
||||
)
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.post(
|
||||
"/gh/v1/git/push",
|
||||
content=body,
|
||||
headers={**_signed("POST", "/gh/v1/git/push", body), "Content-Type": "application/json"},
|
||||
)
|
||||
|
||||
assert resp.status_code == 200, resp.text
|
||||
assert captured["path"] == repo_dir
|
||||
assert captured["slot_uid"] == 2001
|
||||
|
||||
|
||||
@pytest.mark.parametrize("slot_uid", [0, -1, 65536])
|
||||
async def test_git_push_rejects_invalid_slot_uid(proxy_settings: Settings, slot_uid: int) -> None:
|
||||
app = _build_app(proxy_settings)
|
||||
body = (
|
||||
b'{"repo":"octo/widget","workspace_key":"octo__widget__1","branch":"x","expected_head":"'
|
||||
+ (b"0" * 40)
|
||||
+ b'","slot_uid":'
|
||||
+ str(slot_uid).encode()
|
||||
+ b"}"
|
||||
)
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.post(
|
||||
"/gh/v1/git/push",
|
||||
content=body,
|
||||
headers={**_signed("POST", "/gh/v1/git/push", body), "Content-Type": "application/json"},
|
||||
)
|
||||
|
||||
assert resp.status_code == 400
|
||||
assert "slot_uid" in resp.text
|
||||
|
||||
|
||||
async def test_git_push_head_drift(proxy_settings: Settings, upstream_repo: Path) -> None:
|
||||
branch = "farm/abc/drift"
|
||||
_, _ = _stage_workspace(proxy_settings, upstream_repo, "octo/widget", 1, branch)
|
||||
app = _build_app(proxy_settings)
|
||||
fake_head = "0" * 40
|
||||
body = (
|
||||
b'{"repo":"octo/widget","workspace_key":"octo__widget__1","branch":"'
|
||||
+ branch.encode()
|
||||
+ b'","expected_head":"'
|
||||
+ fake_head.encode()
|
||||
+ b'"}'
|
||||
)
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.post(
|
||||
"/gh/v1/git/push",
|
||||
content=body,
|
||||
headers={**_signed("POST", "/gh/v1/git/push", body), "Content-Type": "application/json"},
|
||||
)
|
||||
assert resp.status_code == 409, resp.text
|
||||
assert resp.json()["error"]["kind"] == "head_drift"
|
||||
assert not _bare_has_branch(upstream_repo, branch)
|
||||
|
||||
|
||||
async def test_git_push_workspace_key_mismatch(proxy_settings: Settings) -> None:
|
||||
app = _build_app(proxy_settings)
|
||||
body = (
|
||||
b'{"repo":"octo/widget","workspace_key":"other__repo__1","branch":"x","expected_head":"' + (b"0" * 40) + b'"}'
|
||||
)
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.post(
|
||||
"/gh/v1/git/push",
|
||||
content=body,
|
||||
headers={**_signed("POST", "/gh/v1/git/push", body), "Content-Type": "application/json"},
|
||||
)
|
||||
assert resp.status_code == 400
|
||||
assert "workspace_key" in resp.text
|
||||
|
||||
|
||||
# ============================================================================
|
||||
# Finding 2 — HMAC must bind the raw query string
|
||||
# ============================================================================
|
||||
|
||||
|
||||
async def test_hmac_rejects_query_mutation(proxy_settings: Settings) -> None:
|
||||
"""Sign `/gh/v1/issue?repo=octo/widget&number=1`, replay with number=2.
|
||||
|
||||
The verifier MUST notice the mutated query and 401. Without binding the
|
||||
query into the canonical string this request would sail through with an
|
||||
attacker-chosen target issue.
|
||||
"""
|
||||
captured: list[httpx.Request] = []
|
||||
|
||||
def gh(req: httpx.Request) -> httpx.Response:
|
||||
captured.append(req)
|
||||
return httpx.Response(
|
||||
200,
|
||||
json={
|
||||
"number": int(req.url.params["number"]),
|
||||
"title": "T",
|
||||
"body": "B",
|
||||
"state": "open",
|
||||
"user": {"login": "x"},
|
||||
"labels": [],
|
||||
},
|
||||
)
|
||||
|
||||
app = _build_app(proxy_settings, gh)
|
||||
legit_params = {"repo": "octo/widget", "number": 1}
|
||||
headers = _signed("GET", "/gh/v1/issue", params=legit_params)
|
||||
mutated = {"repo": "octo/widget", "number": 2}
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.get("/gh/v1/issue", params=mutated, headers=headers)
|
||||
assert resp.status_code == 401, resp.text
|
||||
# Upstream GitHub mock MUST NOT have been called — auth failed first.
|
||||
assert captured == []
|
||||
|
||||
|
||||
# ============================================================================
|
||||
# Finding 3 — body must be size-capped BEFORE auth / before full buffer
|
||||
# ============================================================================
|
||||
|
||||
|
||||
async def test_oversized_content_length_rejected_with_413(proxy_settings: Settings) -> None:
|
||||
"""Setting Content-Length above the cap is rejected at 413 cheaply.
|
||||
|
||||
With the fix in place the proxy never reads the (huge) body into memory:
|
||||
we declare CL > max_bytes and the handler aborts immediately. We force
|
||||
a tiny cap so the test stays fast; the production default is 1 MiB.
|
||||
"""
|
||||
proxy_settings.gh_proxy_max_body_bytes = 256 # type: ignore[misc]
|
||||
app = _build_app(proxy_settings, lambda _: httpx.Response(500, json={}))
|
||||
payload = b"x" * 1024
|
||||
headers = {
|
||||
**_signed("POST", "/gh/v1/post_comment", payload),
|
||||
"Content-Type": "application/json",
|
||||
# Lie about CL to prove the early-reject path doesn't read content.
|
||||
"Content-Length": str(1024 * 1024 * 64),
|
||||
}
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.post("/gh/v1/post_comment", content=payload, headers=headers)
|
||||
assert resp.status_code == 413, resp.text
|
||||
|
||||
|
||||
async def test_streamed_body_above_cap_rejected_with_413(proxy_settings: Settings) -> None:
|
||||
"""When Content-Length is honest but > cap, we still 413."""
|
||||
proxy_settings.gh_proxy_max_body_bytes = 64 # type: ignore[misc]
|
||||
app = _build_app(proxy_settings, lambda _: httpx.Response(500, json={}))
|
||||
payload = b'{"repo":"octo/widget","number":1,"body":"' + (b"y" * 200) + b'"}'
|
||||
headers = {
|
||||
**_signed("POST", "/gh/v1/post_comment", payload),
|
||||
"Content-Type": "application/json",
|
||||
}
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.post("/gh/v1/post_comment", content=payload, headers=headers)
|
||||
assert resp.status_code == 413
|
||||
|
||||
|
||||
# ============================================================================
|
||||
# Finding 5 — push refuses attacker-controlled origin (PAT exfil guard)
|
||||
# ============================================================================
|
||||
|
||||
|
||||
async def test_git_push_rejects_attacker_origin(proxy_settings: Settings, upstream_repo: Path) -> None:
|
||||
"""If the worktree's origin is rewritten to a non-github HTTPS URL,
|
||||
the push endpoint MUST refuse with 400 BEFORE invoking `git push` (which
|
||||
would carry the PAT to the attacker's host)."""
|
||||
branch = "farm/abc/evil"
|
||||
repo_dir, head = _stage_workspace(proxy_settings, upstream_repo, "octo/widget", 1, branch)
|
||||
# Simulate the agent rewriting origin inside its sandbox worktree.
|
||||
_git(["-C", str(repo_dir), "remote", "set-url", "origin", "https://evil.example.com/octo/widget.git"], repo_dir)
|
||||
|
||||
app = _build_app(proxy_settings)
|
||||
body = (
|
||||
b'{"repo":"octo/widget","workspace_key":"octo__widget__1","branch":"'
|
||||
+ branch.encode()
|
||||
+ b'","expected_head":"'
|
||||
+ head.encode()
|
||||
+ b'"}'
|
||||
)
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.post(
|
||||
"/gh/v1/git/push",
|
||||
content=body,
|
||||
headers={**_signed("POST", "/gh/v1/git/push", body), "Content-Type": "application/json"},
|
||||
)
|
||||
assert resp.status_code == 400, resp.text
|
||||
# The legit upstream never received the branch — proves push wasn't run.
|
||||
assert not _bare_has_branch(upstream_repo, branch)
|
||||
|
||||
|
||||
async def test_git_push_rejects_origin_with_wrong_repo(proxy_settings: Settings, upstream_repo: Path) -> None:
|
||||
"""github.com host is not enough — owner/repo MUST match the request."""
|
||||
branch = "farm/abc/mismatch"
|
||||
repo_dir, head = _stage_workspace(proxy_settings, upstream_repo, "octo/widget", 1, branch)
|
||||
_git(["-C", str(repo_dir), "remote", "set-url", "origin", "https://github.com/attacker/other.git"], repo_dir)
|
||||
|
||||
app = _build_app(proxy_settings)
|
||||
body = (
|
||||
b'{"repo":"octo/widget","workspace_key":"octo__widget__1","branch":"'
|
||||
+ branch.encode()
|
||||
+ b'","expected_head":"'
|
||||
+ head.encode()
|
||||
+ b'"}'
|
||||
)
|
||||
async with await _async_client(app) as client:
|
||||
resp = await client.post(
|
||||
"/gh/v1/git/push",
|
||||
content=body,
|
||||
headers={**_signed("POST", "/gh/v1/git/push", body), "Content-Type": "application/json"},
|
||||
)
|
||||
assert resp.status_code == 400, resp.text
|
||||
assert not _bare_has_branch(upstream_repo, branch)
|
||||
@@ -0,0 +1,292 @@
|
||||
"""Cancellation primitives on WorkerPool.
|
||||
|
||||
These tests stay at the public-ish surface of `WorkerPool` — they exercise the
|
||||
hook registration contextvar that workers use and verify the dispatcher marks
|
||||
cancelled events as failed with the documented marker. They do NOT spin up a
|
||||
real omp subprocess; that's covered by the integration smoke test.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
|
||||
import pytest
|
||||
|
||||
from robomp.cancellation import (
|
||||
clear_current_event,
|
||||
register_cancel_hook,
|
||||
set_current_event,
|
||||
unregister_cancel_hook,
|
||||
)
|
||||
from robomp.config import Settings
|
||||
from robomp.db import Database, EventRow
|
||||
from robomp.queue import WorkerPool
|
||||
from robomp.slot_pool import SlotPool
|
||||
|
||||
|
||||
class _StubGitHub:
|
||||
"""Sentinel; queue tests don't talk to GitHub."""
|
||||
|
||||
|
||||
class _StubSandbox:
|
||||
"""Sentinel; queue tests don't touch the workspace pool."""
|
||||
|
||||
natives_cache = None
|
||||
|
||||
|
||||
class _StubGitTransport:
|
||||
"""Sentinel; queue tests don't push."""
|
||||
|
||||
|
||||
def _make_pool(settings: Settings, db: Database) -> WorkerPool:
|
||||
return WorkerPool(
|
||||
settings=settings,
|
||||
db=db,
|
||||
github=_StubGitHub(), # type: ignore[arg-type]
|
||||
sandbox=_StubSandbox(), # type: ignore[arg-type]
|
||||
git_transport=_StubGitTransport(), # type: ignore[arg-type]
|
||||
slot_pool=SlotPool(),
|
||||
)
|
||||
|
||||
|
||||
def _row(delivery: str = "d1") -> EventRow:
|
||||
return EventRow(
|
||||
delivery_id=delivery,
|
||||
event_type="issues",
|
||||
repo="octo/widget",
|
||||
issue_key="octo/widget#1",
|
||||
payload={"action": "opened"},
|
||||
received_at="2026-01-01T00:00:00Z",
|
||||
state="running",
|
||||
attempts=1,
|
||||
last_error=None,
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_cancel_fires_hook_armed_by_worker(settings: Settings, db: Database) -> None:
|
||||
"""A worker that armed a hook gets it invoked when cancel_event runs."""
|
||||
pool = _make_pool(settings, db)
|
||||
row = _row()
|
||||
|
||||
fired = asyncio.Event()
|
||||
|
||||
async def fake_worker() -> None:
|
||||
# Mimic _run_event entering its contextvar scope: the helpers below are
|
||||
# what worker.py invokes from inside the asyncio.to_thread call.
|
||||
token = set_current_event(pool, row.delivery_id)
|
||||
try:
|
||||
await asyncio.to_thread(register_cancel_hook, fired.set)
|
||||
# Park until somebody fires the hook.
|
||||
await fired.wait()
|
||||
finally:
|
||||
await asyncio.to_thread(unregister_cancel_hook)
|
||||
clear_current_event(token)
|
||||
|
||||
worker = asyncio.create_task(fake_worker())
|
||||
# Give the worker a tick to register.
|
||||
for _ in range(20):
|
||||
await asyncio.sleep(0)
|
||||
if row.delivery_id in pool._cancel_hooks: # noqa: SLF001 — test inspecting state
|
||||
break
|
||||
assert row.delivery_id in pool._cancel_hooks # noqa: SLF001
|
||||
|
||||
assert await pool.cancel_event(row.delivery_id) is True
|
||||
await asyncio.wait_for(worker, timeout=1.0)
|
||||
assert row.delivery_id in pool._cancelled # noqa: SLF001
|
||||
# Hook is consumed.
|
||||
assert row.delivery_id not in pool._cancel_hooks # noqa: SLF001
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_cancel_before_arm_fires_immediately(settings: Settings, db: Database) -> None:
|
||||
"""Cancelling before the worker arms must still terminate it on register."""
|
||||
pool = _make_pool(settings, db)
|
||||
row = _row("d2")
|
||||
|
||||
# Cancel is requested before any worker has armed a hook.
|
||||
assert await pool.cancel_event(row.delivery_id) is False
|
||||
assert row.delivery_id in pool._cancelled # noqa: SLF001
|
||||
|
||||
# When the worker eventually registers, the hook must fire synchronously.
|
||||
calls: list[int] = []
|
||||
token = set_current_event(pool, row.delivery_id)
|
||||
try:
|
||||
register_cancel_hook(lambda: calls.append(1))
|
||||
finally:
|
||||
clear_current_event(token)
|
||||
|
||||
assert calls == [1]
|
||||
# Late-armed hook is NOT retained; cancel state is one-shot.
|
||||
assert row.delivery_id not in pool._cancel_hooks # noqa: SLF001
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_dispatch_marks_cancelled_event_failed_with_marker(
|
||||
settings: Settings, db: Database, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""A dispatch that observed cancellation marks the row failed + 'cancelled by operator'."""
|
||||
pool = _make_pool(settings, db)
|
||||
|
||||
db.record_event(
|
||||
delivery_id="d3",
|
||||
event_type="issues",
|
||||
repo="octo/widget",
|
||||
issue_key="octo/widget#1",
|
||||
payload={"action": "opened"},
|
||||
state="running",
|
||||
)
|
||||
row = _row("d3")
|
||||
|
||||
async def fake_dispatch(self: WorkerPool, r: EventRow, *, slot_uid: int | None = None) -> None:
|
||||
# Simulate cancellation hitting mid-task and the omp subprocess raising.
|
||||
await pool.cancel_event(r.delivery_id)
|
||||
raise RuntimeError("subprocess died")
|
||||
|
||||
monkeypatch.setattr(WorkerPool, "_dispatch", fake_dispatch)
|
||||
await pool._run_event(row) # noqa: SLF001 — testing the dispatcher branch directly
|
||||
|
||||
stored = db.get_event("d3")
|
||||
assert stored is not None
|
||||
assert stored.state == "failed"
|
||||
assert stored.last_error == "cancelled by operator"
|
||||
# State is cleared for future events.
|
||||
assert row.delivery_id not in pool._cancelled # noqa: SLF001
|
||||
assert row.delivery_id not in pool._cancel_hooks # noqa: SLF001
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_non_cancelled_failure_keeps_real_traceback(
|
||||
settings: Settings, db: Database, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""A garden-variety dispatch failure still records the traceback path."""
|
||||
pool = _make_pool(settings, db)
|
||||
db.record_event(
|
||||
delivery_id="d4",
|
||||
event_type="issues",
|
||||
repo="octo/widget",
|
||||
issue_key="octo/widget#1",
|
||||
payload={"action": "opened"},
|
||||
state="running",
|
||||
)
|
||||
row = _row("d4")
|
||||
|
||||
async def fake_dispatch(self: WorkerPool, r: EventRow, *, slot_uid: int | None = None) -> None:
|
||||
raise ValueError("boom 42")
|
||||
|
||||
monkeypatch.setattr(WorkerPool, "_dispatch", fake_dispatch)
|
||||
await pool._run_event(row) # noqa: SLF001
|
||||
|
||||
stored = db.get_event("d4")
|
||||
assert stored is not None
|
||||
assert stored.state == "failed"
|
||||
assert stored.last_error is not None
|
||||
assert "boom 42" in stored.last_error
|
||||
assert "cancelled by operator" not in stored.last_error
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_event_marks_failed_when_not_shutting_down(
|
||||
settings: Settings, db: Database, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""When `_shutting_down` is False, a dispatch failure still marks the row failed."""
|
||||
pool = _make_pool(settings, db)
|
||||
assert pool._shutting_down is False # noqa: SLF001
|
||||
db.record_event(
|
||||
delivery_id="d5",
|
||||
event_type="issues",
|
||||
repo="octo/widget",
|
||||
issue_key="octo/widget#1",
|
||||
payload={"action": "opened"},
|
||||
state="running",
|
||||
)
|
||||
row = _row("d5")
|
||||
|
||||
async def fake_dispatch(self: WorkerPool, r: EventRow, *, slot_uid: int | None = None) -> None:
|
||||
raise RuntimeError("regular failure")
|
||||
|
||||
monkeypatch.setattr(WorkerPool, "_dispatch", fake_dispatch)
|
||||
await pool._run_event(row) # noqa: SLF001
|
||||
|
||||
stored = db.get_event("d5")
|
||||
assert stored is not None
|
||||
assert stored.state == "failed"
|
||||
assert stored.last_error is not None
|
||||
assert "regular failure" in stored.last_error
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_cancel_unknown_delivery_returns_false(settings: Settings, db: Database) -> None:
|
||||
"""Cancelling an unknown delivery is a no-op that returns False."""
|
||||
pool = _make_pool(settings, db)
|
||||
assert await pool.cancel_event("never-existed") is False
|
||||
# The set still records the request — a later register would fire — but
|
||||
# since no worker is armed, the cancel is harmless.
|
||||
assert "never-existed" in pool._cancelled # noqa: SLF001
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_start_reaps_configured_slot_uids(
|
||||
settings: Settings, db: Database, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
calls: list[int] = []
|
||||
monkeypatch.setattr("robomp.queue._reap_slot", lambda uid: calls.append(uid))
|
||||
pool = WorkerPool(
|
||||
settings=settings,
|
||||
db=db,
|
||||
github=_StubGitHub(), # type: ignore[arg-type]
|
||||
sandbox=_StubSandbox(), # type: ignore[arg-type]
|
||||
git_transport=_StubGitTransport(), # type: ignore[arg-type]
|
||||
slot_pool=SlotPool([2001, 2002]),
|
||||
)
|
||||
|
||||
await pool.start()
|
||||
try:
|
||||
assert sorted(calls) == [2001, 2002]
|
||||
finally:
|
||||
await pool.stop(drain_timeout=0.01, kill_timeout=0.01)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_event_reaps_slot_before_release(
|
||||
settings: Settings, db: Database, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
slot_pool = SlotPool([2001])
|
||||
pool = WorkerPool(
|
||||
settings=settings,
|
||||
db=db,
|
||||
github=_StubGitHub(), # type: ignore[arg-type]
|
||||
sandbox=_StubSandbox(), # type: ignore[arg-type]
|
||||
git_transport=_StubGitTransport(), # type: ignore[arg-type]
|
||||
slot_pool=slot_pool,
|
||||
)
|
||||
db.record_event(
|
||||
delivery_id="d-slot",
|
||||
event_type="issues",
|
||||
repo="octo/widget",
|
||||
issue_key="octo/widget#1",
|
||||
payload={"action": "opened"},
|
||||
state="running",
|
||||
)
|
||||
order: list[tuple[str, int | None]] = []
|
||||
monkeypatch.setattr("robomp.queue._reap_slot", lambda uid: order.append(("reap", uid)))
|
||||
release = slot_pool.release
|
||||
|
||||
def record_release(slot_uid: int | None) -> None:
|
||||
order.append(("release", slot_uid))
|
||||
release(slot_uid)
|
||||
|
||||
monkeypatch.setattr(slot_pool, "release", record_release)
|
||||
|
||||
async def fake_dispatch(self: WorkerPool, r: EventRow, *, slot_uid: int | None = None) -> None:
|
||||
assert r.delivery_id == "d-slot"
|
||||
assert slot_uid == 2001
|
||||
|
||||
monkeypatch.setattr(WorkerPool, "_dispatch", fake_dispatch)
|
||||
|
||||
await pool._run_event(_row("d-slot")) # noqa: SLF001
|
||||
|
||||
stored = db.get_event("d-slot")
|
||||
assert stored is not None
|
||||
assert stored.state == "done"
|
||||
assert order == [("reap", 2001), ("release", 2001)]
|
||||
@@ -0,0 +1,281 @@
|
||||
"""Graceful shutdown drain + kill behavior on WorkerPool.
|
||||
|
||||
These tests poke `WorkerPool` directly: they don't spin up a dispatcher loop
|
||||
or omp subprocess. The contract under test is `stop()`'s drain-then-kill
|
||||
sequence and `_run_event`'s shutting-down branch that leaves the DB row in
|
||||
`running` so `reset_stuck_running()` can requeue it.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
from contextlib import suppress
|
||||
|
||||
import pytest
|
||||
|
||||
from robomp.config import Settings
|
||||
from robomp.db import Database, EventRow
|
||||
from robomp.queue import WorkerPool
|
||||
from robomp.slot_pool import SlotPool
|
||||
|
||||
|
||||
class _StubGitHub:
|
||||
"""Sentinel; queue tests don't talk to GitHub."""
|
||||
|
||||
|
||||
class _StubSandbox:
|
||||
"""Sentinel; queue tests don't touch the workspace pool."""
|
||||
|
||||
natives_cache = None
|
||||
|
||||
|
||||
class _StubGitTransport:
|
||||
"""Sentinel; queue tests don't push."""
|
||||
|
||||
|
||||
def _make_pool(settings: Settings, db: Database) -> WorkerPool:
|
||||
return WorkerPool(
|
||||
settings=settings,
|
||||
db=db,
|
||||
github=_StubGitHub(), # type: ignore[arg-type]
|
||||
sandbox=_StubSandbox(), # type: ignore[arg-type]
|
||||
git_transport=_StubGitTransport(), # type: ignore[arg-type]
|
||||
slot_pool=SlotPool(),
|
||||
)
|
||||
|
||||
|
||||
def _row(delivery: str = "d1") -> EventRow:
|
||||
return EventRow(
|
||||
delivery_id=delivery,
|
||||
event_type="issues",
|
||||
repo="octo/widget",
|
||||
issue_key="octo/widget#1",
|
||||
payload={"action": "opened"},
|
||||
received_at="2026-01-01T00:00:00Z",
|
||||
state="running",
|
||||
attempts=1,
|
||||
last_error=None,
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_non_root_fallback_semaphore_caps_dispatch_concurrency(
|
||||
settings: Settings, db: Database, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
settings.max_concurrency = 1
|
||||
monkeypatch.setattr("robomp.queue.os.geteuid", lambda: 501)
|
||||
|
||||
pool = WorkerPool(
|
||||
settings=settings,
|
||||
db=db,
|
||||
github=_StubGitHub(), # type: ignore[arg-type]
|
||||
sandbox=_StubSandbox(), # type: ignore[arg-type]
|
||||
git_transport=_StubGitTransport(), # type: ignore[arg-type]
|
||||
)
|
||||
|
||||
db.record_event(
|
||||
delivery_id="d-one",
|
||||
event_type="issues",
|
||||
repo="octo/widget",
|
||||
issue_key="octo/widget#1",
|
||||
payload={"action": "opened"},
|
||||
state="running",
|
||||
)
|
||||
db.record_event(
|
||||
delivery_id="d-two",
|
||||
event_type="issues",
|
||||
repo="octo/widget",
|
||||
issue_key="octo/widget#2",
|
||||
payload={"action": "opened"},
|
||||
state="running",
|
||||
)
|
||||
|
||||
dispatch_started = asyncio.Event()
|
||||
release_dispatch = asyncio.Event()
|
||||
started: list[str] = []
|
||||
|
||||
async def blocked_dispatch(self: WorkerPool, row: EventRow, *, slot_uid: int | None = None) -> None:
|
||||
assert slot_uid is None
|
||||
started.append(row.delivery_id)
|
||||
dispatch_started.set()
|
||||
await release_dispatch.wait()
|
||||
|
||||
monkeypatch.setattr(WorkerPool, "_dispatch", blocked_dispatch)
|
||||
|
||||
first = asyncio.create_task(pool._run_event(_row("d-one"))) # noqa: SLF001
|
||||
await asyncio.wait_for(dispatch_started.wait(), timeout=1.0)
|
||||
|
||||
second = asyncio.create_task(pool._run_event(_row("d-two"))) # noqa: SLF001
|
||||
await asyncio.sleep(0)
|
||||
assert started == ["d-one"]
|
||||
|
||||
release_dispatch.set()
|
||||
await asyncio.wait_for(asyncio.gather(first, second), timeout=1.0)
|
||||
assert started == ["d-one", "d-two"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_stop_drains_inflight_within_timeout(settings: Settings, db: Database) -> None:
|
||||
"""A short in-flight task finishes during the drain window; no kill hook needed."""
|
||||
pool = _make_pool(settings, db)
|
||||
|
||||
async def short_coro() -> None:
|
||||
await asyncio.sleep(0.05)
|
||||
|
||||
task = asyncio.create_task(short_coro())
|
||||
pool._inflight_tasks[task] = "d-short" # noqa: SLF001
|
||||
|
||||
await pool.stop(drain_timeout=1.0, kill_timeout=0.1)
|
||||
|
||||
assert pool._shutting_down is True # noqa: SLF001
|
||||
assert task.done()
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_stop_fires_kill_hook_when_drain_exceeds_timeout(settings: Settings, db: Database) -> None:
|
||||
"""When drain times out, stop() pops and runs the registered cancel hook.
|
||||
|
||||
The DB row stays `running` because `_run_event` (not exercised here) is
|
||||
the only path that mutates state, and even when triggered post-kill the
|
||||
shutting_down flag suppresses `mark_event(..., 'failed')`.
|
||||
"""
|
||||
pool = _make_pool(settings, db)
|
||||
db.record_event(
|
||||
delivery_id="d-blocked",
|
||||
event_type="issues",
|
||||
repo="octo/widget",
|
||||
issue_key="octo/widget#1",
|
||||
payload={"action": "opened"},
|
||||
state="running",
|
||||
)
|
||||
|
||||
hook_called = asyncio.Event()
|
||||
pool._cancel_hooks["d-blocked"] = hook_called.set # noqa: SLF001
|
||||
|
||||
never = asyncio.Event()
|
||||
|
||||
async def _park() -> None:
|
||||
await never.wait()
|
||||
|
||||
blocked = asyncio.create_task(_park())
|
||||
pool._inflight_tasks[blocked] = "d-blocked" # noqa: SLF001
|
||||
|
||||
await pool.stop(drain_timeout=0.05, kill_timeout=0.05)
|
||||
|
||||
assert hook_called.is_set()
|
||||
stored = db.get_event("d-blocked")
|
||||
assert stored is not None
|
||||
assert stored.state == "running"
|
||||
|
||||
blocked.cancel()
|
||||
with suppress(asyncio.CancelledError):
|
||||
await blocked
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_event_skips_mark_event_when_shutting_down(
|
||||
settings: Settings, db: Database, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""During shutdown, a dispatch exception on a deliberately-cancelled delivery leaves the row untouched."""
|
||||
pool = _make_pool(settings, db)
|
||||
pool._shutting_down = True # noqa: SLF001
|
||||
pool._shutdown_cancelled.add("d-shutdown") # noqa: SLF001
|
||||
|
||||
db.record_event(
|
||||
delivery_id="d-shutdown",
|
||||
event_type="issues",
|
||||
repo="octo/widget",
|
||||
issue_key="octo/widget#1",
|
||||
payload={"action": "opened"},
|
||||
state="running",
|
||||
)
|
||||
|
||||
async def fake_dispatch(self: WorkerPool, r: EventRow, *, slot_uid: int | None = None) -> None:
|
||||
raise RuntimeError("omp died")
|
||||
|
||||
monkeypatch.setattr(WorkerPool, "_dispatch", fake_dispatch)
|
||||
await pool._run_event(_row("d-shutdown")) # noqa: SLF001
|
||||
|
||||
stored = db.get_event("d-shutdown")
|
||||
assert stored is not None
|
||||
assert stored.state == "running"
|
||||
assert stored.last_error is None
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_stop_cancels_hookless_inflight_task(settings: Settings, db: Database) -> None:
|
||||
"""A task claimed but stuck pre-hook MUST be cancelled by stop(), not allowed to spawn omp.
|
||||
|
||||
Reproduces the P1 finding: pre-fix, stop()'s kill phase iterated cancel
|
||||
hooks only, so an in-flight task without a hook (still waiting on the
|
||||
semaphore or inside RpcClient.__enter__) was left running and could
|
||||
proceed to spawn a fresh subprocess after stop() returned.
|
||||
"""
|
||||
pool = _make_pool(settings, db)
|
||||
|
||||
reached_spawn = False
|
||||
pre_hook_started = asyncio.Event()
|
||||
|
||||
async def stuck_pre_hook() -> None:
|
||||
nonlocal reached_spawn
|
||||
pre_hook_started.set()
|
||||
# Simulate waiting on a slow resource (semaphore / RpcClient.__enter__);
|
||||
# we never get a chance to register a cancel hook.
|
||||
try:
|
||||
await asyncio.sleep(5.0)
|
||||
except asyncio.CancelledError:
|
||||
raise
|
||||
# Pre-fix: this line was reachable after stop() returned.
|
||||
reached_spawn = True
|
||||
|
||||
task = asyncio.create_task(stuck_pre_hook())
|
||||
pool._inflight_tasks[task] = "d-hookless" # noqa: SLF001
|
||||
await asyncio.wait_for(pre_hook_started.wait(), timeout=1.0)
|
||||
|
||||
await pool.stop(drain_timeout=0.05, kill_timeout=0.2)
|
||||
|
||||
# Give the event loop a tick for cancellation to settle, then assert.
|
||||
await asyncio.sleep(0)
|
||||
assert task.done(), "stop() must terminate hookless in-flight tasks"
|
||||
assert task.cancelled(), "hookless task must be cancelled, not left running"
|
||||
assert reached_spawn is False, "task body must not progress past stop()"
|
||||
assert "d-hookless" in pool._shutdown_cancelled # noqa: SLF001
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_event_marks_failed_for_unrelated_failure_during_drain(
|
||||
settings: Settings, db: Database, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""A dispatch that fails for its own reasons during the drain window MUST still mark failed.
|
||||
|
||||
Reproduces the P2 finding: pre-fix, `_shutting_down=True` alone gated
|
||||
the suppression branch, so any exception raised during the drain
|
||||
window was masked and the row was silently requeued on the next
|
||||
start(). After the fix, only deliveries in `_shutdown_cancelled`
|
||||
(the ones stop() actually interrupted) get the suppression.
|
||||
"""
|
||||
pool = _make_pool(settings, db)
|
||||
pool._shutting_down = True # noqa: SLF001
|
||||
# Crucially: this delivery is NOT in `_shutdown_cancelled` — stop()
|
||||
# never targeted it. Its failure is its own.
|
||||
|
||||
db.record_event(
|
||||
delivery_id="d-real-fail",
|
||||
event_type="issues",
|
||||
repo="octo/widget",
|
||||
issue_key="octo/widget#1",
|
||||
payload={"action": "opened"},
|
||||
state="running",
|
||||
)
|
||||
|
||||
async def fake_dispatch(self: WorkerPool, r: EventRow, *, slot_uid: int | None = None) -> None:
|
||||
raise RuntimeError("genuine bug, not shutdown")
|
||||
|
||||
monkeypatch.setattr(WorkerPool, "_dispatch", fake_dispatch)
|
||||
await pool._run_event(_row("d-real-fail")) # noqa: SLF001
|
||||
|
||||
stored = db.get_event("d-real-fail")
|
||||
assert stored is not None
|
||||
assert stored.state == "failed", "non-shutdown failure during drain must mark failed"
|
||||
assert stored.last_error is not None
|
||||
assert "genuine bug, not shutdown" in stored.last_error
|
||||
File diff suppressed because it is too large
Load Diff
File diff suppressed because it is too large
Load Diff
@@ -0,0 +1,56 @@
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
|
||||
import pytest
|
||||
|
||||
from robomp.slot_pool import SlotPool
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_empty_pool_is_noop() -> None:
|
||||
pool = SlotPool()
|
||||
|
||||
assert await pool.acquire() is None
|
||||
pool.release(None)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_acquire_release_reuses_uid() -> None:
|
||||
pool = SlotPool([2001])
|
||||
|
||||
assert await pool.acquire() == 2001
|
||||
pool.release(2001)
|
||||
|
||||
assert await pool.acquire() == 2001
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_double_release_rejected() -> None:
|
||||
pool = SlotPool([2001])
|
||||
|
||||
slot_uid = await pool.acquire()
|
||||
pool.release(slot_uid)
|
||||
|
||||
with pytest.raises(ValueError, match="not acquired"):
|
||||
pool.release(slot_uid)
|
||||
|
||||
|
||||
def test_duplicate_slots_rejected() -> None:
|
||||
with pytest.raises(ValueError, match="unique"):
|
||||
SlotPool([2001, 2001])
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_concurrent_acquire_waits_until_release() -> None:
|
||||
pool = SlotPool([2001])
|
||||
|
||||
first_slot_uid = await pool.acquire()
|
||||
second_acquire = asyncio.create_task(pool.acquire())
|
||||
await asyncio.sleep(0)
|
||||
|
||||
assert not second_acquire.done()
|
||||
|
||||
pool.release(first_slot_uid)
|
||||
|
||||
assert await second_acquire == 2001
|
||||
@@ -0,0 +1,51 @@
|
||||
"""Verify pragmas survive the payload round-trip from server → durable queue → tasks."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from robomp.tasks import _directive_from_payload
|
||||
|
||||
|
||||
def test_directive_from_payload_parses_pragmas() -> None:
|
||||
directive = _directive_from_payload(
|
||||
{
|
||||
"_robomp_directive": {
|
||||
"body": "do the thing",
|
||||
"author": "can1357",
|
||||
"pragmas": [["model", "gpt"], ["thinking", "low"]],
|
||||
}
|
||||
}
|
||||
)
|
||||
assert directive is not None
|
||||
assert directive.body == "do the thing"
|
||||
assert directive.author == "can1357"
|
||||
assert directive.pragmas == (("model", "gpt"), ("thinking", "low"))
|
||||
|
||||
|
||||
def test_directive_from_payload_missing_pragmas_is_empty_tuple() -> None:
|
||||
directive = _directive_from_payload({"_robomp_directive": {"body": "x", "author": "can1357"}})
|
||||
assert directive is not None
|
||||
assert directive.pragmas == ()
|
||||
|
||||
|
||||
def test_directive_from_payload_drops_malformed_pragma_entries() -> None:
|
||||
directive = _directive_from_payload(
|
||||
{
|
||||
"_robomp_directive": {
|
||||
"body": "x",
|
||||
"author": "can1357",
|
||||
"pragmas": [
|
||||
["model", "gpt"],
|
||||
["bad"], # wrong arity
|
||||
[1, "v"], # non-string key
|
||||
"string-instead-of-pair",
|
||||
],
|
||||
}
|
||||
}
|
||||
)
|
||||
assert directive is not None
|
||||
assert directive.pragmas == (("model", "gpt"),)
|
||||
|
||||
|
||||
def test_directive_from_payload_returns_none_for_missing_directive() -> None:
|
||||
assert _directive_from_payload({}) is None
|
||||
assert _directive_from_payload({"_robomp_directive": "not-a-mapping"}) is None
|
||||
@@ -0,0 +1,770 @@
|
||||
"""Resume-aware behavior of `worker._run_rpc_blocking`.
|
||||
|
||||
These tests swap `robomp.worker.RpcClient` for a recording fake so we can
|
||||
observe the `extra_args` and `set_todos` decisions the driver takes based on
|
||||
whether the workspace's omp session directory already holds a JSONL transcript.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import stat
|
||||
from pathlib import Path
|
||||
from types import SimpleNamespace
|
||||
|
||||
import pytest
|
||||
|
||||
from robomp import worker
|
||||
from robomp.config import Settings
|
||||
|
||||
|
||||
class _FakeRpcClient:
|
||||
instances: list[_FakeRpcClient] = []
|
||||
|
||||
def __init__(self, **kwargs):
|
||||
self.kwargs = kwargs
|
||||
self.set_todos_calls: list[list[dict]] = []
|
||||
self.get_todos_calls = 0
|
||||
self.stop_calls = 0
|
||||
self.mark_closed_calls: list[BaseException] = []
|
||||
_FakeRpcClient.instances.append(self)
|
||||
|
||||
def __enter__(self):
|
||||
return self
|
||||
|
||||
def __exit__(self, exc_type, exc, tb):
|
||||
return False
|
||||
|
||||
def install_headless_ui(self) -> None:
|
||||
pass
|
||||
|
||||
def on_tool_execution_end(self, _cb) -> None:
|
||||
pass
|
||||
|
||||
def on_message_update(self, _cb) -> None:
|
||||
pass
|
||||
|
||||
def stop(self) -> None:
|
||||
self.stop_calls += 1
|
||||
|
||||
def _mark_closed(self, error: BaseException) -> None:
|
||||
self.mark_closed_calls.append(error)
|
||||
|
||||
def set_todos(self, phases):
|
||||
self.set_todos_calls.append(phases)
|
||||
|
||||
def get_todos(self):
|
||||
self.get_todos_calls += 1
|
||||
return ()
|
||||
|
||||
def prompt_and_wait(self, prompt, timeout):
|
||||
if not hasattr(self, "prompts"):
|
||||
self.prompts: list[str] = []
|
||||
self.prompts.append(prompt)
|
||||
hook = getattr(self, "on_prompt", None)
|
||||
if hook is not None:
|
||||
hook(self, prompt)
|
||||
|
||||
class _Turn:
|
||||
messages: list = []
|
||||
events: list = []
|
||||
assistant_text: str = "ok"
|
||||
|
||||
return _Turn()
|
||||
|
||||
|
||||
_SEEDED_PHASES = [
|
||||
{
|
||||
"id": "p1",
|
||||
"name": "Reproduce",
|
||||
"tasks": [
|
||||
{
|
||||
"id": "t1",
|
||||
"content": "do it",
|
||||
"status": "pending",
|
||||
"notes": "",
|
||||
"details": "",
|
||||
}
|
||||
],
|
||||
}
|
||||
]
|
||||
|
||||
|
||||
def _make_inputs(
|
||||
tmp_path: Path, settings: Settings, *, session_has_jsonl: bool, slot_uid: int | None = None
|
||||
) -> tuple[worker.TaskInputs, SimpleNamespace]:
|
||||
root = tmp_path / "workspace"
|
||||
root.mkdir()
|
||||
session_dir = root / "session"
|
||||
session_dir.mkdir()
|
||||
if session_has_jsonl:
|
||||
(session_dir / "foo.jsonl").write_text("{}\n", encoding="utf-8")
|
||||
repo_dir = root / "repo"
|
||||
repo_dir.mkdir()
|
||||
|
||||
workspace = SimpleNamespace(
|
||||
root=root,
|
||||
session_dir=session_dir,
|
||||
repo_dir=repo_dir,
|
||||
branch="robomp/issue-1",
|
||||
)
|
||||
repo = SimpleNamespace(full_name="acme/widgets", owner="acme", name="widgets")
|
||||
issue = SimpleNamespace(repo="acme/widgets", number=1, title="bug")
|
||||
|
||||
db = SimpleNamespace(set_event_model=lambda _did, _model: None, get_issue=lambda _key: None)
|
||||
github = SimpleNamespace()
|
||||
|
||||
inputs = worker.TaskInputs(
|
||||
settings=settings,
|
||||
db=db, # type: ignore[arg-type]
|
||||
github=github, # type: ignore[arg-type]
|
||||
git_transport=SimpleNamespace(), # type: ignore[arg-type]
|
||||
repo=repo, # type: ignore[arg-type]
|
||||
issue=issue, # type: ignore[arg-type]
|
||||
workspace=workspace, # type: ignore[arg-type]
|
||||
delivery_id="d-test",
|
||||
attempts=0,
|
||||
slot_uid=slot_uid,
|
||||
)
|
||||
bindings = SimpleNamespace(
|
||||
workspace=workspace,
|
||||
repo=repo,
|
||||
issue=issue,
|
||||
issue_key=f"{repo.full_name}#{issue.number}",
|
||||
abort=None,
|
||||
)
|
||||
return inputs, bindings
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _reset_fake() -> None:
|
||||
_FakeRpcClient.instances.clear()
|
||||
|
||||
|
||||
@pytest.fixture(autouse=True)
|
||||
def _patch_worker(monkeypatch: pytest.MonkeyPatch, tmp_path: Path) -> None:
|
||||
monkeypatch.setattr("robomp.worker.RpcClient", _FakeRpcClient)
|
||||
monkeypatch.setattr("robomp.worker._AGENT_HOME_STAGE", tmp_path / "missing-agent-home-stage")
|
||||
monkeypatch.setattr("robomp.worker.host_tools.build", lambda _b: ())
|
||||
monkeypatch.setattr(
|
||||
"robomp.worker.persona.system_append",
|
||||
lambda *, repo, issue, workspace: "SYS",
|
||||
)
|
||||
monkeypatch.setattr(
|
||||
"robomp.worker.persona.seed_phases",
|
||||
lambda _kind: [dict(p) for p in _SEEDED_PHASES],
|
||||
)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_rpc_passes_continue_when_session_jsonl_present(tmp_path: Path, settings: Settings) -> None:
|
||||
inputs, bindings = _make_inputs(tmp_path, settings, session_has_jsonl=True)
|
||||
loop = asyncio.new_event_loop()
|
||||
try:
|
||||
worker._run_rpc_blocking(
|
||||
inputs,
|
||||
task_kind="triage_issue",
|
||||
prompt="x",
|
||||
loop=loop,
|
||||
bindings=bindings, # type: ignore[arg-type]
|
||||
)
|
||||
finally:
|
||||
loop.close()
|
||||
assert _FakeRpcClient.instances[0].kwargs["extra_args"] == ("--continue",)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_rpc_omits_continue_when_session_empty(
|
||||
tmp_path: Path, settings: Settings, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
agent_home = tmp_path / "agent-home"
|
||||
agent_home.mkdir()
|
||||
monkeypatch.setattr(worker, "_AGENT_HOME", agent_home)
|
||||
|
||||
inputs, bindings = _make_inputs(tmp_path, settings, session_has_jsonl=False)
|
||||
loop = asyncio.new_event_loop()
|
||||
try:
|
||||
worker._run_rpc_blocking(
|
||||
inputs,
|
||||
task_kind="triage_issue",
|
||||
prompt="x",
|
||||
loop=loop,
|
||||
bindings=bindings, # type: ignore[arg-type]
|
||||
)
|
||||
finally:
|
||||
loop.close()
|
||||
assert _FakeRpcClient.instances[0].kwargs["extra_args"] == ()
|
||||
client_kwargs = _FakeRpcClient.instances[0].kwargs
|
||||
assert client_kwargs["env"]["HOME"] == str(agent_home)
|
||||
assert client_kwargs["env"]["GITHUB_TOKEN"] == ""
|
||||
assert client_kwargs["env"]["GITHUB_WEBHOOK_SECRET"] == ""
|
||||
assert client_kwargs["env"]["ROBOMP_REPLAY_TOKEN"] == ""
|
||||
assert client_kwargs["env"]["ROBOMP_GH_PROXY_HMAC_KEY"] == ""
|
||||
assert client_kwargs["user"] is None
|
||||
assert client_kwargs["group"] is None
|
||||
assert client_kwargs["extra_groups"] is None
|
||||
|
||||
|
||||
def test_build_extra_env_stages_agent_home(tmp_path: Path, settings: Settings, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
stage_home = tmp_path / "agent-home-stage"
|
||||
agent_home = tmp_path / "agent-home"
|
||||
monkeypatch.setattr(worker, "_AGENT_HOME_STAGE", stage_home)
|
||||
monkeypatch.setattr(worker, "_AGENT_HOME", agent_home)
|
||||
|
||||
agent_dir = stage_home / ".agent"
|
||||
agent_rules_dir = agent_dir / "rules"
|
||||
omp_agent_dir = stage_home / ".omp" / "agent"
|
||||
agent_rules_dir.mkdir(parents=True)
|
||||
omp_agent_dir.mkdir(parents=True)
|
||||
(agent_dir / "AGENTS.md").write_text("agent instructions\n", encoding="utf-8")
|
||||
(agent_rules_dir / "rule.md").write_text("rule\n", encoding="utf-8")
|
||||
(omp_agent_dir / "models.yml").write_text("models: []\n", encoding="utf-8")
|
||||
|
||||
env = worker._build_extra_env(settings)
|
||||
|
||||
assert env["HOME"] == str(agent_home)
|
||||
assert (agent_home / ".agent" / "AGENTS.md").is_file()
|
||||
assert (agent_home / ".agent" / "rules" / "rule.md").is_file()
|
||||
assert (agent_home / ".omp" / "agent" / "models.yml").is_file()
|
||||
assert (agent_home / ".agent").stat().st_mode & 0o777 == 0o755
|
||||
assert (agent_home / ".agent" / "AGENTS.md").stat().st_mode & 0o777 == 0o644
|
||||
assert (agent_home / ".agent" / "rules").stat().st_mode & 0o777 == 0o755
|
||||
assert (agent_home / ".agent" / "rules" / "rule.md").stat().st_mode & 0o777 == 0o644
|
||||
assert (agent_home / ".omp" / "agent").stat().st_mode & 0o777 == 0o755
|
||||
assert (agent_home / ".omp" / "agent" / "models.yml").stat().st_mode & 0o777 == 0o644
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_rpc_omits_home_when_agent_home_absent(
|
||||
tmp_path: Path, settings: Settings, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
monkeypatch.setattr(worker, "_AGENT_HOME", tmp_path / "missing-agent-home")
|
||||
|
||||
inputs, bindings = _make_inputs(tmp_path, settings, session_has_jsonl=False)
|
||||
loop = asyncio.new_event_loop()
|
||||
try:
|
||||
worker._run_rpc_blocking(
|
||||
inputs,
|
||||
task_kind="triage_issue",
|
||||
prompt="x",
|
||||
loop=loop,
|
||||
bindings=bindings, # type: ignore[arg-type]
|
||||
)
|
||||
finally:
|
||||
loop.close()
|
||||
client_kwargs = _FakeRpcClient.instances[0].kwargs
|
||||
assert "HOME" not in client_kwargs["env"]
|
||||
assert client_kwargs["env"]["GITHUB_TOKEN"] == ""
|
||||
assert client_kwargs["env"]["GITHUB_WEBHOOK_SECRET"] == ""
|
||||
assert client_kwargs["env"]["ROBOMP_REPLAY_TOKEN"] == ""
|
||||
assert client_kwargs["env"]["ROBOMP_GH_PROXY_HMAC_KEY"] == ""
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_rpc_uses_workspace_xdg_dirs_without_slot(tmp_path: Path, settings: Settings) -> None:
|
||||
inputs, bindings = _make_inputs(tmp_path, settings, session_has_jsonl=False, slot_uid=None)
|
||||
loop = asyncio.new_event_loop()
|
||||
try:
|
||||
worker._run_rpc_blocking(
|
||||
inputs,
|
||||
task_kind="triage_issue",
|
||||
prompt="x",
|
||||
loop=loop,
|
||||
bindings=bindings, # type: ignore[arg-type]
|
||||
)
|
||||
finally:
|
||||
loop.close()
|
||||
|
||||
env = _FakeRpcClient.instances[0].kwargs["env"]
|
||||
xdg_root = inputs.workspace.root / ".omp-xdg"
|
||||
for key in ("XDG_DATA_HOME", "XDG_STATE_HOME", "XDG_CACHE_HOME"):
|
||||
path = Path(env[key])
|
||||
assert path.is_relative_to(xdg_root)
|
||||
assert (path / "omp").is_dir()
|
||||
tmpdir = inputs.workspace.root / ".omp-tmp"
|
||||
assert env["TMPDIR"] == str(tmpdir)
|
||||
assert env["TMP"] == str(tmpdir)
|
||||
assert env["TEMP"] == str(tmpdir)
|
||||
assert env["GIT_CONFIG_COUNT"] == "1"
|
||||
assert env["GIT_CONFIG_KEY_0"] == "safe.directory"
|
||||
assert env["GIT_CONFIG_VALUE_0"] == str(inputs.workspace.repo_dir)
|
||||
assert env["GIT_AUTHOR_NAME"] == settings.resolved_author_name
|
||||
assert env["GIT_AUTHOR_EMAIL"] == settings.git_author_email
|
||||
assert env["GIT_COMMITTER_NAME"] == settings.resolved_author_name
|
||||
assert env["GIT_COMMITTER_EMAIL"] == settings.git_author_email
|
||||
assert tmpdir.is_dir()
|
||||
assert stat.S_IMODE(tmpdir.stat().st_mode) == 0o700
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_rpc_uses_workspace_xdg_dirs_for_slot_without_chown(
|
||||
tmp_path: Path, settings: Settings, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
chown_calls: list[tuple[Path, int, int]] = []
|
||||
monkeypatch.setattr("robomp.sandbox.platform.system", lambda: "Linux")
|
||||
monkeypatch.setattr("robomp.sandbox.os.geteuid", lambda: 0)
|
||||
monkeypatch.setattr("robomp.sandbox.os.chown", lambda path, uid, gid: chown_calls.append((Path(path), uid, gid)))
|
||||
|
||||
inputs, bindings = _make_inputs(tmp_path, settings, session_has_jsonl=False, slot_uid=2001)
|
||||
loop = asyncio.new_event_loop()
|
||||
try:
|
||||
worker._run_rpc_blocking(
|
||||
inputs,
|
||||
task_kind="triage_issue",
|
||||
prompt="x",
|
||||
loop=loop,
|
||||
bindings=bindings, # type: ignore[arg-type]
|
||||
)
|
||||
finally:
|
||||
loop.close()
|
||||
|
||||
env = _FakeRpcClient.instances[0].kwargs["env"]
|
||||
for key in ("XDG_DATA_HOME", "XDG_STATE_HOME", "XDG_CACHE_HOME"):
|
||||
base = Path(env[key])
|
||||
assert base.is_dir()
|
||||
assert (base / "omp").is_dir()
|
||||
assert Path(env["BUN_INSTALL_CACHE_DIR"]).is_dir()
|
||||
assert chown_calls == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_rpc_skips_set_todos_on_resumed_triage(tmp_path: Path, settings: Settings) -> None:
|
||||
inputs, bindings = _make_inputs(tmp_path, settings, session_has_jsonl=True)
|
||||
loop = asyncio.new_event_loop()
|
||||
try:
|
||||
worker._run_rpc_blocking(
|
||||
inputs,
|
||||
task_kind="triage_issue",
|
||||
prompt="x",
|
||||
loop=loop,
|
||||
bindings=bindings, # type: ignore[arg-type]
|
||||
)
|
||||
finally:
|
||||
loop.close()
|
||||
assert _FakeRpcClient.instances[0].set_todos_calls == []
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_rpc_seeds_todos_on_fresh_triage(tmp_path: Path, settings: Settings) -> None:
|
||||
inputs, bindings = _make_inputs(tmp_path, settings, session_has_jsonl=False)
|
||||
loop = asyncio.new_event_loop()
|
||||
try:
|
||||
worker._run_rpc_blocking(
|
||||
inputs,
|
||||
task_kind="triage_issue",
|
||||
prompt="x",
|
||||
loop=loop,
|
||||
bindings=bindings, # type: ignore[arg-type]
|
||||
)
|
||||
finally:
|
||||
loop.close()
|
||||
calls = _FakeRpcClient.instances[0].set_todos_calls
|
||||
assert len(calls) == 1
|
||||
assert calls[0] == _SEEDED_PHASES
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_rpc_merges_todos_on_followup_with_resume(tmp_path: Path, settings: Settings) -> None:
|
||||
inputs, bindings = _make_inputs(tmp_path, settings, session_has_jsonl=True)
|
||||
loop = asyncio.new_event_loop()
|
||||
try:
|
||||
worker._run_rpc_blocking(
|
||||
inputs,
|
||||
task_kind="handle_comment",
|
||||
prompt="x",
|
||||
loop=loop,
|
||||
bindings=bindings, # type: ignore[arg-type]
|
||||
)
|
||||
finally:
|
||||
loop.close()
|
||||
client = _FakeRpcClient.instances[0]
|
||||
assert client.get_todos_calls == 1
|
||||
assert len(client.set_todos_calls) == 1
|
||||
assert len(client.set_todos_calls[0]) == len(_SEEDED_PHASES)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_rpc_passes_slot_uid_user_slot_group_and_omp_extra_group(tmp_path: Path, settings: Settings) -> None:
|
||||
inputs, bindings = _make_inputs(tmp_path, settings, session_has_jsonl=False, slot_uid=2001)
|
||||
loop = asyncio.new_event_loop()
|
||||
try:
|
||||
worker._run_rpc_blocking(
|
||||
inputs,
|
||||
task_kind="triage_issue",
|
||||
prompt="x",
|
||||
loop=loop,
|
||||
bindings=bindings, # type: ignore[arg-type]
|
||||
)
|
||||
finally:
|
||||
loop.close()
|
||||
client_kwargs = _FakeRpcClient.instances[0].kwargs
|
||||
assert client_kwargs["user"] == 2001
|
||||
assert client_kwargs["group"] == 2001
|
||||
assert client_kwargs["extra_groups"] == ["omp"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_rpc_arms_hard_timeout_timer(
|
||||
tmp_path: Path, settings: Settings, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
timers = []
|
||||
|
||||
class FakeTimer:
|
||||
def __init__(self, interval, function):
|
||||
self.interval = interval
|
||||
self.function = function
|
||||
self.daemon = False
|
||||
self.started = False
|
||||
self.cancelled = False
|
||||
timers.append(self)
|
||||
|
||||
def start(self) -> None:
|
||||
self.started = True
|
||||
|
||||
def cancel(self) -> None:
|
||||
self.cancelled = True
|
||||
|
||||
monkeypatch.setattr("robomp.worker.threading.Timer", FakeTimer)
|
||||
settings.task_timeout_seconds = 3.0
|
||||
settings.task_timeout_hard_grace_seconds = 7.0
|
||||
inputs, bindings = _make_inputs(tmp_path, settings, session_has_jsonl=False)
|
||||
loop = asyncio.new_event_loop()
|
||||
try:
|
||||
worker._run_rpc_blocking(
|
||||
inputs,
|
||||
task_kind="triage_issue",
|
||||
prompt="x",
|
||||
loop=loop,
|
||||
bindings=bindings, # type: ignore[arg-type]
|
||||
)
|
||||
finally:
|
||||
loop.close()
|
||||
|
||||
assert len(timers) == 1
|
||||
timer = timers[0]
|
||||
assert timer.interval == 10.0
|
||||
assert timer.daemon is True
|
||||
assert timer.started is True
|
||||
assert timer.cancelled is True
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_rpc_hard_timeout_stops_client_and_fails(
|
||||
tmp_path: Path, settings: Settings, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
class FiringTimer:
|
||||
def __init__(self, interval, function):
|
||||
self.interval = interval
|
||||
self.function = function
|
||||
self.daemon = False
|
||||
self.cancelled = False
|
||||
|
||||
def start(self) -> None:
|
||||
self.function()
|
||||
|
||||
def cancel(self) -> None:
|
||||
self.cancelled = True
|
||||
|
||||
monkeypatch.setattr("robomp.worker.threading.Timer", FiringTimer)
|
||||
inputs, bindings = _make_inputs(tmp_path, settings, session_has_jsonl=False)
|
||||
loop = asyncio.new_event_loop()
|
||||
try:
|
||||
with pytest.raises(TimeoutError, match="hard timeout"):
|
||||
worker._run_rpc_blocking(
|
||||
inputs,
|
||||
task_kind="triage_issue",
|
||||
prompt="x",
|
||||
loop=loop,
|
||||
bindings=bindings, # type: ignore[arg-type]
|
||||
)
|
||||
finally:
|
||||
loop.close()
|
||||
|
||||
fake = _FakeRpcClient.instances[0]
|
||||
assert fake.stop_calls == 1
|
||||
# `_cancel_hook` (used by both manual cancel and hard timeout) MUST also call
|
||||
# `_mark_closed` to unblock `_wait_for_agent_end` — `stop()` alone leaves
|
||||
# `_closed_error` unset (omp_rpc bug), so the worker would hang otherwise.
|
||||
assert len(fake.mark_closed_calls) == 1
|
||||
from omp_rpc import RpcProcessExitError
|
||||
|
||||
assert isinstance(fake.mark_closed_calls[0], RpcProcessExitError)
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_rpc_cancel_hook_stops_and_marks_closed(
|
||||
tmp_path: Path, settings: Settings, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""The cancel hook registered with `register_cancel_hook` must call both
|
||||
`client.stop()` AND `client._mark_closed()`. The latter is the workaround for
|
||||
an upstream omp_rpc bug where `stop()` does not set `_closed_error`, leaving
|
||||
`_wait_for_agent_end` blocked until timeout."""
|
||||
captured: list = []
|
||||
monkeypatch.setattr("robomp.worker.register_cancel_hook", lambda hook: captured.append(hook))
|
||||
monkeypatch.setattr("robomp.worker.unregister_cancel_hook", lambda: None)
|
||||
|
||||
inputs, bindings = _make_inputs(tmp_path, settings, session_has_jsonl=False)
|
||||
loop = asyncio.new_event_loop()
|
||||
try:
|
||||
worker._run_rpc_blocking(
|
||||
inputs,
|
||||
task_kind="triage_issue",
|
||||
prompt="x",
|
||||
loop=loop,
|
||||
bindings=bindings, # type: ignore[arg-type]
|
||||
)
|
||||
finally:
|
||||
loop.close()
|
||||
|
||||
assert len(captured) == 1
|
||||
hook = captured[0]
|
||||
fake = _FakeRpcClient.instances[0]
|
||||
pre_stop = fake.stop_calls
|
||||
hook() # Simulate the API/worker firing the cancel
|
||||
assert fake.stop_calls == pre_stop + 1
|
||||
assert len(fake.mark_closed_calls) == 1
|
||||
from omp_rpc import RpcProcessExitError
|
||||
|
||||
assert isinstance(fake.mark_closed_calls[0], RpcProcessExitError)
|
||||
assert "cancelled by operator" in str(fake.mark_closed_calls[0])
|
||||
|
||||
|
||||
class _ClassifiedRow:
|
||||
"""Stand-in for `db.IssueRow` carrying just `.classification`."""
|
||||
|
||||
def __init__(self, classification: str | None) -> None:
|
||||
self.classification = classification
|
||||
|
||||
|
||||
def _make_inputs_with_classification(
|
||||
tmp_path: Path,
|
||||
settings: Settings,
|
||||
*,
|
||||
classification: str | None,
|
||||
) -> tuple[worker.TaskInputs, SimpleNamespace]:
|
||||
inputs, bindings = _make_inputs(tmp_path, settings, session_has_jsonl=True)
|
||||
row = _ClassifiedRow(classification) if classification else None
|
||||
inputs.db.get_issue = lambda _key: row # type: ignore[attr-defined]
|
||||
bindings.db = inputs.db # tools_called check uses inputs.db.get_issue
|
||||
return inputs, bindings
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_rpc_sends_reminder_when_pr_class_quits_early(tmp_path: Path, settings: Settings) -> None:
|
||||
"""`bug` classified turn that never calls a terminal tool gets a reminder."""
|
||||
inputs, bindings = _make_inputs_with_classification(tmp_path, settings, classification="bug")
|
||||
loop = asyncio.new_event_loop()
|
||||
try:
|
||||
worker._run_rpc_blocking(
|
||||
inputs,
|
||||
task_kind="triage_issue",
|
||||
prompt="kickoff",
|
||||
loop=loop,
|
||||
bindings=bindings, # type: ignore[arg-type]
|
||||
)
|
||||
finally:
|
||||
loop.close()
|
||||
fake = _FakeRpcClient.instances[0]
|
||||
# kickoff + 2 reminders (default ROBOMP_TASK_COMPLETION_MAX_REMINDERS=2)
|
||||
assert len(fake.prompts) == 1 + settings.task_completion_max_reminders
|
||||
assert fake.prompts[0] == "kickoff"
|
||||
assert all("terminal action" in p.lower() or "open the pr" in p.lower() for p in fake.prompts[1:])
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_rpc_stops_reminding_after_terminal_tool(tmp_path: Path, settings: Settings) -> None:
|
||||
"""A reminder turn that fires `gh_open_pr` halts the loop."""
|
||||
inputs, bindings = _make_inputs_with_classification(tmp_path, settings, classification="bug")
|
||||
|
||||
# First turn returns with no terminal tool; first reminder causes the
|
||||
# agent to "call" gh_open_pr — simulated by mutating the worker's
|
||||
# tools_called set via the on_prompt hook on the next prompt.
|
||||
def _on_prompt(client: _FakeRpcClient, prompt: str) -> None:
|
||||
if len(client.prompts) == 2: # this is the first reminder
|
||||
# Mimic a tool_end firing during the reminder turn by writing
|
||||
# into the closure set the driver tracks. We can't reach it
|
||||
# directly; instead trip the abort path? No — use the public
|
||||
# contract: tool_end fires through on_tool_execution_end. The
|
||||
# driver registers the callback before prompt_and_wait, so we
|
||||
# replay it here.
|
||||
for cb in client._tool_end_callbacks:
|
||||
cb(SimpleNamespace(tool_name="gh_open_pr", result={}))
|
||||
|
||||
# Capture the registered tool_end callback on the fake.
|
||||
original_on_tool_end = _FakeRpcClient.on_tool_execution_end
|
||||
|
||||
def _record_tool_end(self, cb) -> None:
|
||||
self._tool_end_callbacks = getattr(self, "_tool_end_callbacks", [])
|
||||
self._tool_end_callbacks.append(cb)
|
||||
|
||||
_FakeRpcClient.on_tool_execution_end = _record_tool_end # type: ignore[assignment]
|
||||
try:
|
||||
_FakeRpcClient.on_prompt = staticmethod(_on_prompt) # type: ignore[attr-defined]
|
||||
loop = asyncio.new_event_loop()
|
||||
try:
|
||||
worker._run_rpc_blocking(
|
||||
inputs,
|
||||
task_kind="triage_issue",
|
||||
prompt="kickoff",
|
||||
loop=loop,
|
||||
bindings=bindings, # type: ignore[arg-type]
|
||||
)
|
||||
finally:
|
||||
loop.close()
|
||||
finally:
|
||||
_FakeRpcClient.on_tool_execution_end = original_on_tool_end # type: ignore[assignment]
|
||||
delattr(_FakeRpcClient, "on_prompt")
|
||||
|
||||
fake = _FakeRpcClient.instances[0]
|
||||
# kickoff + 1 reminder; second reminder NOT sent because gh_open_pr fired.
|
||||
assert len(fake.prompts) == 2, fake.prompts
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_rpc_skips_reminder_for_non_pr_classification(tmp_path: Path, settings: Settings) -> None:
|
||||
"""`question` classified turns are not enforced — no reminder."""
|
||||
inputs, bindings = _make_inputs_with_classification(tmp_path, settings, classification="question")
|
||||
loop = asyncio.new_event_loop()
|
||||
try:
|
||||
worker._run_rpc_blocking(
|
||||
inputs,
|
||||
task_kind="triage_issue",
|
||||
prompt="kickoff",
|
||||
loop=loop,
|
||||
bindings=bindings, # type: ignore[arg-type]
|
||||
)
|
||||
finally:
|
||||
loop.close()
|
||||
fake = _FakeRpcClient.instances[0]
|
||||
assert len(fake.prompts) == 1
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_run_rpc_skips_reminder_when_unclassified(tmp_path: Path, settings: Settings) -> None:
|
||||
"""No classification (agent quit before classify_issue) → no reminder."""
|
||||
inputs, bindings = _make_inputs_with_classification(tmp_path, settings, classification=None)
|
||||
loop = asyncio.new_event_loop()
|
||||
try:
|
||||
worker._run_rpc_blocking(
|
||||
inputs,
|
||||
task_kind="triage_issue",
|
||||
prompt="kickoff",
|
||||
loop=loop,
|
||||
bindings=bindings, # type: ignore[arg-type]
|
||||
)
|
||||
finally:
|
||||
loop.close()
|
||||
fake = _FakeRpcClient.instances[0]
|
||||
assert len(fake.prompts) == 1
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# Natives-cache capture-on-success
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
class _RecordingNativesCache:
|
||||
"""Test double for `NativesCache`: records `capture` calls, optionally
|
||||
raises so we can verify exception swallowing."""
|
||||
|
||||
def __init__(self, *, raise_on_capture: bool = False) -> None:
|
||||
self.capture_calls: list[tuple[str, str, Path]] = []
|
||||
self.raise_on_capture = raise_on_capture
|
||||
|
||||
def capture(self, repo: str, key: str, native_dir: Path, **_kwargs) -> Path | None:
|
||||
self.capture_calls.append((repo, key, native_dir))
|
||||
if self.raise_on_capture:
|
||||
raise RuntimeError("simulated cache failure")
|
||||
return native_dir
|
||||
|
||||
|
||||
def _make_capture_inputs(
|
||||
tmp_path: Path,
|
||||
settings: Settings,
|
||||
*,
|
||||
cache: _RecordingNativesCache | None,
|
||||
with_native_artifacts: bool,
|
||||
) -> worker.TaskInputs:
|
||||
"""Build a `TaskInputs` whose workspace optionally has built natives."""
|
||||
inputs, _ = _make_inputs(tmp_path, settings, session_has_jsonl=False)
|
||||
# Replace the SimpleNamespace workspace with one carrying the fields
|
||||
# `_capture_natives_cache` needs (workspace_key + repo_full_name).
|
||||
ws = SimpleNamespace(
|
||||
root=inputs.workspace.root,
|
||||
session_dir=inputs.workspace.session_dir,
|
||||
repo_dir=inputs.workspace.repo_dir,
|
||||
branch=inputs.workspace.branch,
|
||||
workspace_key="acme__widgets__1",
|
||||
repo_full_name="acme/widgets",
|
||||
)
|
||||
if with_native_artifacts:
|
||||
native_dir = ws.repo_dir / "packages" / "natives" / "native"
|
||||
native_dir.mkdir(parents=True)
|
||||
(native_dir / "pi_natives.linux-arm64.node").write_bytes(b"ELFx")
|
||||
(native_dir / "index.d.ts").write_text("")
|
||||
(native_dir / "index.js").write_text("")
|
||||
(native_dir / "embedded-addon.js").write_text("")
|
||||
return worker.TaskInputs(
|
||||
settings=settings,
|
||||
db=inputs.db,
|
||||
github=inputs.github,
|
||||
git_transport=inputs.git_transport,
|
||||
repo=inputs.repo,
|
||||
issue=inputs.issue,
|
||||
workspace=ws, # type: ignore[arg-type]
|
||||
delivery_id=inputs.delivery_id,
|
||||
attempts=inputs.attempts,
|
||||
slot_uid=inputs.slot_uid,
|
||||
natives_cache=cache, # type: ignore[arg-type]
|
||||
)
|
||||
|
||||
|
||||
def test_capture_natives_cache_no_op_without_cache(tmp_path: Path, settings: Settings) -> None:
|
||||
inputs = _make_capture_inputs(tmp_path, settings, cache=None, with_native_artifacts=True)
|
||||
# Just must not raise.
|
||||
worker._capture_natives_cache(inputs)
|
||||
|
||||
|
||||
def test_capture_natives_cache_skips_without_artifacts(tmp_path: Path, settings: Settings) -> None:
|
||||
cache = _RecordingNativesCache()
|
||||
inputs = _make_capture_inputs(tmp_path, settings, cache=cache, with_native_artifacts=False)
|
||||
worker._capture_natives_cache(inputs)
|
||||
# No artifacts → no key compute, no capture.
|
||||
assert cache.capture_calls == []
|
||||
|
||||
|
||||
def test_capture_natives_cache_swallows_key_compute_failure(
|
||||
tmp_path: Path, settings: Settings, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
cache = _RecordingNativesCache()
|
||||
inputs = _make_capture_inputs(tmp_path, settings, cache=cache, with_native_artifacts=True)
|
||||
# Repo dir is not a git repo → natives_compute_key raises.
|
||||
# Already true for the SimpleNamespace workspace (repo_dir is plain tmp dir).
|
||||
worker._capture_natives_cache(inputs)
|
||||
assert cache.capture_calls == []
|
||||
|
||||
|
||||
def test_capture_natives_cache_swallows_capture_exception(
|
||||
tmp_path: Path, settings: Settings, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
cache = _RecordingNativesCache(raise_on_capture=True)
|
||||
inputs = _make_capture_inputs(tmp_path, settings, cache=cache, with_native_artifacts=True)
|
||||
# Bypass git: stub the key compute so capture is reached.
|
||||
monkeypatch.setattr(worker, "natives_compute_key", lambda _repo_dir: "deadbeef")
|
||||
# Must not propagate the RuntimeError.
|
||||
worker._capture_natives_cache(inputs)
|
||||
assert len(cache.capture_calls) == 1
|
||||
|
||||
|
||||
def test_capture_natives_cache_records_on_success(
|
||||
tmp_path: Path, settings: Settings, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
cache = _RecordingNativesCache()
|
||||
inputs = _make_capture_inputs(tmp_path, settings, cache=cache, with_native_artifacts=True)
|
||||
monkeypatch.setattr(worker, "natives_compute_key", lambda _repo_dir: "cafef00d")
|
||||
worker._capture_natives_cache(inputs)
|
||||
assert len(cache.capture_calls) == 1
|
||||
repo, key, native_dir = cache.capture_calls[0]
|
||||
assert repo == "acme/widgets"
|
||||
assert key == "cafef00d"
|
||||
assert native_dir == inputs.workspace.repo_dir / "packages" / "natives" / "native"
|
||||
@@ -0,0 +1,80 @@
|
||||
"""Worker-side pragma resolution: model + thinking overrides."""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import pytest
|
||||
|
||||
from robomp.config import Settings, reset_settings_cache
|
||||
from robomp.worker import DirectiveInfo, _resolve_pragma_overrides
|
||||
|
||||
|
||||
@pytest.fixture
|
||||
def settings_with_pool(monkeypatch: pytest.MonkeyPatch, env: dict[str, str]) -> Settings:
|
||||
monkeypatch.setenv(
|
||||
"ROBOMP_MODEL",
|
||||
"p-anthropic/claude-sonnet-4-6,p-openai/gpt-5.5,p-openai/gpt-5.5-mini",
|
||||
)
|
||||
reset_settings_cache()
|
||||
return Settings() # type: ignore[call-arg]
|
||||
|
||||
|
||||
def test_no_directive_means_no_override(settings_with_pool: Settings) -> None:
|
||||
assert _resolve_pragma_overrides(None, settings_with_pool) == (None, None)
|
||||
|
||||
|
||||
def test_directive_without_pragmas_means_no_override(settings_with_pool: Settings) -> None:
|
||||
directive = DirectiveInfo(body="run it", author="can1357")
|
||||
assert _resolve_pragma_overrides(directive, settings_with_pool) == (None, None)
|
||||
|
||||
|
||||
def test_model_pragma_resolves_to_pool_entry(settings_with_pool: Settings) -> None:
|
||||
directive = DirectiveInfo(body="run", author="can1357", pragmas=(("model", "gpt"),))
|
||||
model_override, thinking_override = _resolve_pragma_overrides(directive, settings_with_pool)
|
||||
assert model_override == "p-openai/gpt-5.5"
|
||||
assert thinking_override is None
|
||||
|
||||
|
||||
def test_model_alias_exact_short_name(settings_with_pool: Settings) -> None:
|
||||
directive = DirectiveInfo(body="run", author="can1357", pragmas=(("model", "gpt-5.5-mini"),))
|
||||
model_override, _ = _resolve_pragma_overrides(directive, settings_with_pool)
|
||||
assert model_override == "p-openai/gpt-5.5-mini"
|
||||
|
||||
|
||||
def test_unmatched_model_alias_falls_back_to_random_pick(settings_with_pool: Settings) -> None:
|
||||
directive = DirectiveInfo(body="run", author="can1357", pragmas=(("model", "qwen"),))
|
||||
model_override, _ = _resolve_pragma_overrides(directive, settings_with_pool)
|
||||
assert model_override is None
|
||||
|
||||
|
||||
def test_thinking_pragma_normalized(settings_with_pool: Settings) -> None:
|
||||
directive = DirectiveInfo(body="run", author="can1357", pragmas=(("thinking", "LOW"),))
|
||||
model_override, thinking_override = _resolve_pragma_overrides(directive, settings_with_pool)
|
||||
assert model_override is None
|
||||
assert thinking_override == "low"
|
||||
|
||||
|
||||
def test_unknown_thinking_level_dropped(settings_with_pool: Settings) -> None:
|
||||
directive = DirectiveInfo(body="run", author="can1357", pragmas=(("thinking", "ultra"),))
|
||||
_, thinking_override = _resolve_pragma_overrides(directive, settings_with_pool)
|
||||
assert thinking_override is None
|
||||
|
||||
|
||||
def test_both_pragmas_resolved_together(settings_with_pool: Settings) -> None:
|
||||
directive = DirectiveInfo(
|
||||
body="run",
|
||||
author="can1357",
|
||||
pragmas=(("model", "claude"), ("thinking", "medium")),
|
||||
)
|
||||
model_override, thinking_override = _resolve_pragma_overrides(directive, settings_with_pool)
|
||||
assert model_override == "p-anthropic/claude-sonnet-4-6"
|
||||
assert thinking_override == "medium"
|
||||
|
||||
|
||||
def test_last_value_wins_for_duplicate_keys(settings_with_pool: Settings) -> None:
|
||||
directive = DirectiveInfo(
|
||||
body="run",
|
||||
author="can1357",
|
||||
pragmas=(("model", "claude"), ("model", "gpt")),
|
||||
)
|
||||
model_override, _ = _resolve_pragma_overrides(directive, settings_with_pool)
|
||||
assert model_override == "p-openai/gpt-5.5"
|
||||
@@ -0,0 +1,201 @@
|
||||
"""Gated end-to-end smoke test.
|
||||
|
||||
Runs only when ROBOMP_INTEGRATION=1 and `omp` is available on PATH (or via
|
||||
ROBOMP_OMP_COMMAND). Spins up:
|
||||
|
||||
- a local bare git repo with a trivial failing test,
|
||||
- a fake GitHub API via httpx.MockTransport that records comments + PRs,
|
||||
- a real `omp --mode rpc` subprocess driven by `worker.run_task`.
|
||||
|
||||
Asserts that triage_issue produces:
|
||||
- at least one issue comment,
|
||||
- one PR matching the body template,
|
||||
- a pushed branch on the bare repo,
|
||||
- an `opened` row in sqlite.
|
||||
"""
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
import asyncio
|
||||
import json
|
||||
import os
|
||||
import subprocess
|
||||
from pathlib import Path
|
||||
from typing import Any
|
||||
|
||||
import httpx
|
||||
import pytest
|
||||
|
||||
INTEGRATION = os.environ.get("ROBOMP_INTEGRATION") == "1"
|
||||
|
||||
pytestmark = pytest.mark.skipif(
|
||||
not INTEGRATION,
|
||||
reason="ROBOMP_INTEGRATION=1 required to run the omp-backed smoke test",
|
||||
)
|
||||
|
||||
|
||||
def _git(cwd: Path, *args: str, check: bool = True) -> subprocess.CompletedProcess[str]:
|
||||
env = os.environ | {
|
||||
"GIT_AUTHOR_NAME": "t",
|
||||
"GIT_AUTHOR_EMAIL": "t@t",
|
||||
"GIT_COMMITTER_NAME": "t",
|
||||
"GIT_COMMITTER_EMAIL": "t@t",
|
||||
}
|
||||
return subprocess.run(["git", *args], cwd=str(cwd), check=check, capture_output=True, text=True, env=env)
|
||||
|
||||
|
||||
def _seed_failing_repo(tmp_path: Path) -> Path:
|
||||
bare = tmp_path / "upstream.git"
|
||||
bare.mkdir()
|
||||
_git(bare.parent, "init", "--initial-branch=main", "--bare", str(bare))
|
||||
seed = tmp_path / "seed"
|
||||
seed.mkdir()
|
||||
_git(seed, "init", "--initial-branch=main")
|
||||
(seed / "test.js").write_text(
|
||||
"const assert = require('assert');\n"
|
||||
"// FIXME: this assertion is wrong; the answer is 4.\n"
|
||||
"assert.strictEqual(2 + 2, 5);\n"
|
||||
)
|
||||
(seed / "README.md").write_text("toy repo\n")
|
||||
_git(seed, "add", ".")
|
||||
_git(seed, "commit", "-m", "init")
|
||||
_git(seed, "remote", "add", "origin", str(bare))
|
||||
_git(seed, "push", "origin", "main")
|
||||
return bare
|
||||
|
||||
|
||||
def test_triage_end_to_end(tmp_path: Path, monkeypatch: pytest.MonkeyPatch) -> None:
|
||||
from robomp.config import Settings, reset_settings_cache
|
||||
from robomp.db import Database
|
||||
from robomp.github_client import GitHubClient
|
||||
from robomp.sandbox import LocalGitTransport, SandboxManager
|
||||
from robomp.tasks import triage_issue
|
||||
|
||||
bare = _seed_failing_repo(tmp_path)
|
||||
|
||||
monkeypatch.setenv("ROBOMP_GH_PROXY_URL", "http://gh-proxy.invalid:8081")
|
||||
monkeypatch.setenv("ROBOMP_GH_PROXY_HMAC_KEY", "test-hmac-key-aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa")
|
||||
monkeypatch.setenv("GITHUB_TOKEN", "")
|
||||
monkeypatch.setenv("GITHUB_WEBHOOK_SECRET", "secret")
|
||||
monkeypatch.setenv("ROBOMP_BOT_LOGIN", "robomp-bot")
|
||||
monkeypatch.setenv("ROBOMP_REPO_ALLOWLIST", "octo/widget")
|
||||
monkeypatch.setenv("ROBOMP_WORKSPACE_ROOT", str(tmp_path / "workspaces"))
|
||||
monkeypatch.setenv("ROBOMP_SQLITE_PATH", str(tmp_path / "robomp.sqlite"))
|
||||
monkeypatch.setenv("ROBOMP_LOG_DIR", str(tmp_path / "logs"))
|
||||
monkeypatch.setenv("ROBOMP_TASK_TIMEOUT_SECONDS", "300")
|
||||
reset_settings_cache()
|
||||
cfg = Settings() # type: ignore[call-arg]
|
||||
cfg.ensure_paths()
|
||||
|
||||
# Fake GitHub: capture POSTs, serve repo/issue/comments GETs.
|
||||
comments: list[dict[str, Any]] = []
|
||||
prs: list[dict[str, Any]] = []
|
||||
next_comment_id = [100]
|
||||
|
||||
def handler(request: httpx.Request) -> httpx.Response:
|
||||
path = request.url.path
|
||||
method = request.method
|
||||
if method == "GET" and path == "/repos/octo/widget":
|
||||
return httpx.Response(
|
||||
200,
|
||||
json={
|
||||
"full_name": "octo/widget",
|
||||
"default_branch": "main",
|
||||
"clone_url": str(bare),
|
||||
"private": False,
|
||||
},
|
||||
)
|
||||
if method == "GET" and path == "/repos/octo/widget/issues/1":
|
||||
return httpx.Response(
|
||||
200,
|
||||
json={
|
||||
"number": 1,
|
||||
"title": "2+2 should be 4",
|
||||
"body": "Running `node test.js` exits non-zero because the assertion claims 2+2 is 5.",
|
||||
"state": "open",
|
||||
"user": {"login": "alice"},
|
||||
"labels": [],
|
||||
},
|
||||
)
|
||||
if method == "GET" and path == "/repos/octo/widget/issues/1/comments":
|
||||
return httpx.Response(200, json=comments)
|
||||
if method == "POST" and path == "/repos/octo/widget/issues/1/comments":
|
||||
body = json.loads(request.content)
|
||||
next_comment_id[0] += 1
|
||||
comment = {
|
||||
"id": next_comment_id[0],
|
||||
"user": {"login": "robomp-bot"},
|
||||
"body": body["body"],
|
||||
"created_at": "now",
|
||||
}
|
||||
comments.append(comment)
|
||||
return httpx.Response(201, json=comment)
|
||||
if method == "POST" and path == "/repos/octo/widget/pulls":
|
||||
body = json.loads(request.content)
|
||||
pr = {
|
||||
"number": 7,
|
||||
"html_url": "https://example.invalid/octo/widget/pull/7",
|
||||
"head": {"ref": body["head"]},
|
||||
"base": {"ref": body["base"]},
|
||||
"state": "open",
|
||||
"title": body["title"],
|
||||
"body": body["body"],
|
||||
}
|
||||
prs.append(pr)
|
||||
return httpx.Response(201, json=pr)
|
||||
return httpx.Response(404, json={"message": f"unmocked {method} {path}"})
|
||||
|
||||
transport = httpx.MockTransport(handler)
|
||||
|
||||
payload = {
|
||||
"action": "opened",
|
||||
"issue": {
|
||||
"number": 1,
|
||||
"title": "2+2 should be 4",
|
||||
"body": "Running `node test.js` exits non-zero because the assertion claims 2+2 is 5.",
|
||||
"state": "open",
|
||||
"user": {"login": "alice"},
|
||||
"labels": [],
|
||||
},
|
||||
"repository": {
|
||||
"full_name": "octo/widget",
|
||||
"default_branch": "main",
|
||||
"clone_url": str(bare),
|
||||
"private": False,
|
||||
},
|
||||
}
|
||||
|
||||
async def _go() -> None:
|
||||
db = Database(cfg.sqlite_path)
|
||||
github = GitHubClient("ghp_test", transport=transport)
|
||||
sandbox = SandboxManager(cfg.workspace_root)
|
||||
await triage_issue(
|
||||
settings=cfg,
|
||||
db=db,
|
||||
github=github,
|
||||
git_transport=LocalGitTransport(token=None),
|
||||
sandbox=sandbox,
|
||||
payload=payload,
|
||||
delivery_id="smoke-test",
|
||||
)
|
||||
row = db.get_issue("octo/widget#1")
|
||||
assert row is not None, "issue row missing"
|
||||
assert row.state in {"opened"}, f"unexpected state {row.state}"
|
||||
db.close()
|
||||
|
||||
asyncio.run(_go())
|
||||
|
||||
assert prs, "no PR opened"
|
||||
pr = prs[0]
|
||||
for section in ("## Repro", "## Cause", "## Fix", "## Verification"):
|
||||
assert section in pr["body"], f"PR body missing {section}"
|
||||
assert "Fixes #1" in pr["body"]
|
||||
# Branch should be pushed to the bare repo.
|
||||
refs = subprocess.run(
|
||||
["git", "-C", str(bare), "for-each-ref", "--format=%(refname)"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
check=True,
|
||||
)
|
||||
assert any(r.startswith("refs/heads/farm/") for r in refs.stdout.splitlines()), refs.stdout
|
||||
assert comments, "expected at least one comment"
|
||||
Reference in New Issue
Block a user