mirror of
https://github.com/rennf93/roboco.git
synced 2026-08-03 07:23:24 +02:00
fix(api): unconditional /git/file window cap; defer Telegram sends after commit; docs/map periodic re-index
_compute_file_range now caps any resolved window at _FILE_MAX_LINES instead of only the exact whole-file shape, closing the near-whole-file bypass. Telegram sends ride a generalized after-commit outbox (defer_after_commit over the F107 machinery) so a slow Bot API can no longer hold the caller's transaction open; TelegramClient grows an abstract close(). The KB update loop iterates AUTO_INDEX_DIRS so docs/map edits re-index without a restart. PR-label application catches all exceptions per its never-raises contract, and pr_merge's CEO-only message names the resolved branch.
This commit is contained in:
@@ -12,11 +12,14 @@ from typing import TYPE_CHECKING, cast
|
||||
from uuid import UUID, uuid4
|
||||
|
||||
import pytest
|
||||
from roboco.config import settings
|
||||
from roboco.db.tables import AgentTable, NotificationTable
|
||||
from roboco.events import Event, EventType
|
||||
from roboco.models import AgentRole, AgentStatus, NotificationPriority, NotificationType
|
||||
from roboco.models.base import Team
|
||||
from roboco.services.notification_delivery import get_notification_delivery_service
|
||||
from roboco.services.telegram_client import TelegramSendResult
|
||||
from roboco.services.telegram_credentials import TelegramCredentialsData
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from sqlalchemy.ext.asyncio import AsyncSession
|
||||
@@ -251,3 +254,99 @@ async def test_acknowledge_rollback_drops_phantom(
|
||||
await _await_drain(db_session)
|
||||
|
||||
assert bus.published == []
|
||||
|
||||
|
||||
# --- Telegram send rides the same after-commit outbox, not an inline await ---
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_notify_telegram_send_deferred_to_after_commit(
|
||||
db_session: AsyncSession, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""``_notify_telegram`` must not block the caller's open transaction on
|
||||
the Telegram Bot API call — the network send is deferred to the same
|
||||
after-commit outbox the bus publish above uses, and never fires on a
|
||||
rollback."""
|
||||
monkeypatch.setattr(settings, "telegram_enabled", True)
|
||||
monkeypatch.setattr(settings, "panel_base_url", "")
|
||||
|
||||
creds = TelegramCredentialsData(bot_token="t", chat_id="1")
|
||||
|
||||
class _FakeCredsService:
|
||||
async def get_decrypted(self) -> TelegramCredentialsData:
|
||||
return creds
|
||||
|
||||
monkeypatch.setattr(
|
||||
"roboco.services.telegram_credentials.get_telegram_credentials_service",
|
||||
lambda _session: _FakeCredsService(),
|
||||
)
|
||||
|
||||
sent: list[str] = []
|
||||
|
||||
class _FakeTelegramClient:
|
||||
async def send_message(self, text: str) -> TelegramSendResult:
|
||||
sent.append(text)
|
||||
return TelegramSendResult(sent=True)
|
||||
|
||||
async def close(self) -> None:
|
||||
pass
|
||||
|
||||
monkeypatch.setattr(
|
||||
"roboco.services.telegram_client.build_telegram_client",
|
||||
lambda _creds, **_kwargs: _FakeTelegramClient(),
|
||||
)
|
||||
|
||||
service = get_notification_delivery_service(db_session)
|
||||
await service._notify_telegram(task_id=uuid4(), subject="Hello CEO")
|
||||
|
||||
# Pre-commit: the network send must not have fired yet.
|
||||
assert sent == []
|
||||
|
||||
await db_session.commit()
|
||||
await _await_drain(db_session)
|
||||
|
||||
assert sent == ["Hello CEO"]
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_notify_telegram_rollback_drops_send(
|
||||
db_session: AsyncSession, monkeypatch: pytest.MonkeyPatch
|
||||
) -> None:
|
||||
"""A rollback drops the deferred Telegram send — it never fires for a
|
||||
notification whose row never became durable."""
|
||||
monkeypatch.setattr(settings, "telegram_enabled", True)
|
||||
monkeypatch.setattr(settings, "panel_base_url", "")
|
||||
|
||||
creds = TelegramCredentialsData(bot_token="t", chat_id="1")
|
||||
|
||||
class _FakeCredsService:
|
||||
async def get_decrypted(self) -> TelegramCredentialsData:
|
||||
return creds
|
||||
|
||||
monkeypatch.setattr(
|
||||
"roboco.services.telegram_credentials.get_telegram_credentials_service",
|
||||
lambda _session: _FakeCredsService(),
|
||||
)
|
||||
|
||||
sent: list[str] = []
|
||||
|
||||
class _FakeTelegramClient:
|
||||
async def send_message(self, text: str) -> TelegramSendResult:
|
||||
sent.append(text)
|
||||
return TelegramSendResult(sent=True)
|
||||
|
||||
async def close(self) -> None:
|
||||
pass
|
||||
|
||||
monkeypatch.setattr(
|
||||
"roboco.services.telegram_client.build_telegram_client",
|
||||
lambda _creds, **_kwargs: _FakeTelegramClient(),
|
||||
)
|
||||
|
||||
service = get_notification_delivery_service(db_session)
|
||||
await service._notify_telegram(task_id=uuid4(), subject="Hello CEO")
|
||||
|
||||
await db_session.rollback()
|
||||
await _await_drain(db_session)
|
||||
|
||||
assert sent == []
|
||||
|
||||
@@ -52,3 +52,19 @@ class TestComputeFileRange:
|
||||
total=0, line=None, context=10, start=None, end=None
|
||||
)
|
||||
assert (s, e_, trunc) == (1, 1, False)
|
||||
|
||||
def test_near_whole_file_explicit_range_still_capped(self) -> None:
|
||||
# start=1, end=total-1 is not the exact-whole-file shape, but the
|
||||
# resolved window is still oversized and must be capped.
|
||||
total = 50000
|
||||
s, e_, trunc = _compute_file_range(
|
||||
total=total, line=None, context=10, start=1, end=total - 1
|
||||
)
|
||||
assert (s, e_, trunc) == (1, _FILE_MAX_LINES, True)
|
||||
|
||||
def test_oversized_line_context_window_is_capped(self) -> None:
|
||||
total = 10000
|
||||
s, e_, trunc = _compute_file_range(
|
||||
total=total, line=5000, context=3000, start=None, end=None
|
||||
)
|
||||
assert (s, e_, trunc) == (2000, 2000 + _FILE_MAX_LINES - 1, True)
|
||||
|
||||
@@ -714,6 +714,43 @@ async def test_ensure_base_falls_back_to_default_when_create_fails() -> None:
|
||||
assert out == "master"
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# _apply_pr_labels / _ensure_label_exists: never raise, even on a non-httpx
|
||||
# error — labeling must not surface as a spurious tool failure after the PR
|
||||
# itself was already created.
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
|
||||
def _non_httpx_raising_client() -> MagicMock:
|
||||
"""An AsyncClient whose POST raises a plain (non-httpx) exception."""
|
||||
fake_client = MagicMock()
|
||||
fake_client.__aenter__ = AsyncMock(return_value=fake_client)
|
||||
fake_client.__aexit__ = AsyncMock(return_value=False)
|
||||
fake_client.post = AsyncMock(side_effect=RuntimeError("boom"))
|
||||
return fake_client
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_ensure_label_exists_swallows_non_httpx_error() -> None:
|
||||
svc = _service()
|
||||
with patch(
|
||||
"roboco.services.git.httpx.AsyncClient",
|
||||
return_value=_non_httpx_raising_client(),
|
||||
):
|
||||
await svc._ensure_label_exists("acme", "repo", "tok", "cell/backend")
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_apply_pr_labels_swallows_non_httpx_error() -> None:
|
||||
svc = _service()
|
||||
_bind(svc, "_ensure_label_exists", AsyncMock())
|
||||
with patch(
|
||||
"roboco.services.git.httpx.AsyncClient",
|
||||
return_value=_non_httpx_raising_client(),
|
||||
):
|
||||
await svc._apply_pr_labels("acme", "repo", "tok", 11, ["cell/backend"])
|
||||
|
||||
|
||||
# ---------------------------------------------------------------------------
|
||||
# pr_merge: returns merge commit dict
|
||||
# ---------------------------------------------------------------------------
|
||||
|
||||
@@ -9,11 +9,15 @@ branch is needed — every docs/map/*.md rides index_documentation like docs/rag
|
||||
|
||||
from __future__ import annotations
|
||||
|
||||
from typing import TYPE_CHECKING
|
||||
from unittest.mock import AsyncMock, patch
|
||||
|
||||
import pytest
|
||||
from roboco.services.optimal import IndexingReport, OptimalService
|
||||
|
||||
if TYPE_CHECKING:
|
||||
from pathlib import Path
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_auto_index_docs_includes_map_subdir() -> None:
|
||||
@@ -30,3 +34,28 @@ async def test_auto_index_docs_includes_map_subdir() -> None:
|
||||
names = [c.args[1] for c in mock.call_args_list]
|
||||
assert "rag" in names
|
||||
assert "map" in names
|
||||
|
||||
|
||||
@pytest.mark.asyncio
|
||||
async def test_periodic_check_reindexes_modified_map_file(tmp_path: Path) -> None:
|
||||
"""The periodic loop must watch docs/map too, not just docs/rag — a
|
||||
modified docs/map file with an already-tracked (stale) mtime is picked
|
||||
up by _check_for_updates and routed through _index_doc_file with the
|
||||
"map" dir name, exactly like the one-shot path resolves it."""
|
||||
docs_root = tmp_path / "docs"
|
||||
(docs_root / "rag").mkdir(parents=True)
|
||||
(docs_root / "map").mkdir(parents=True)
|
||||
map_file = docs_root / "map" / "CLAUDE.md"
|
||||
map_file.write_text("# initial")
|
||||
|
||||
svc = object.__new__(OptimalService)
|
||||
svc._docs_root = docs_root
|
||||
# Simulate this file was already indexed at startup with a stale mtime,
|
||||
# so the next scan sees it as modified.
|
||||
svc._file_mtimes = {str(map_file): 0.0}
|
||||
|
||||
mock = AsyncMock()
|
||||
with patch.object(svc, "_index_doc_file", new=mock):
|
||||
await svc._check_for_updates()
|
||||
|
||||
mock.assert_awaited_once_with(map_file, "map")
|
||||
|
||||
Reference in New Issue
Block a user