100% Coverage

tests/unit/services/test_audit.py

@@ -6,6 +6,7 @@ from uuid import uuid4
 
 import pytest
 from roboco.models.audit import (
+    AuditEventType,
     PermissionDenialContext,
     StateTransitionDenialContext,
 )
@@ -113,8 +114,6 @@ async def test_log_notification_denial(svc: AuditService) -> None:
 
 @pytest.mark.asyncio
 async def test_log_security_event(svc: AuditService) -> None:
-    from roboco.models.audit import AuditEventType
-
     await svc.log_security_event(
         event_type=AuditEventType.PERMISSION_DENIED,
         agent_id=str(uuid4()),
@@ -143,6 +142,36 @@ async def test_log_agent_event(svc: AuditService) -> None:
     )
 
 
+@pytest.mark.asyncio
+async def test_log_pm_override(svc: AuditService) -> None:
+    await svc.log_pm_override(
+        agent_id=uuid4(),
+        task_id=uuid4(),
+        action="complete_with_cancelled_subtasks",
+        justification="subtasks were superseded",
+        cancelled_subtask_ids=[str(uuid4())],
+    )
+
+
+@pytest.mark.asyncio
+async def test_log_pm_override_no_subtasks(svc: AuditService) -> None:
+    await svc.log_pm_override(
+        agent_id=uuid4(),
+        task_id=uuid4(),
+        action="force_complete",
+        justification="all done",
+    )
+
+
+@pytest.mark.asyncio
+async def test_log_task_event_basic(svc: AuditService) -> None:
+    await svc.log_task_event(
+        event_type="task.created",
+        task_id=uuid4(),
+        agent_id=uuid4(),
+    )
+
+
 # ---------------------------------------------------------------------------
 # _AuditEvent dataclass
 # ---------------------------------------------------------------------------

tests/unit/services/test_audit_real_query.py

@@ -31,9 +31,11 @@ from uuid import UUID, uuid4
 
 import pytest
 import pytest_asyncio
+from roboco.db import base as db_base
 from roboco.db import base as roboco_db_base
 from roboco.db.tables import AgentTable
 from roboco.models.base import AgentRole, AgentStatus
+from roboco.seeds import initial_data as seeds
 from roboco.services.audit import AuditService
 from sqlalchemy.ext.asyncio import async_sessionmaker
 
@@ -207,3 +209,188 @@ async def test_has_recent_tracing_gap_respects_since_window(
         since=future_since,
     )
     assert result is False
+
+
+# ---------------------------------------------------------------------------
+# get_recent_events — query method exercises severity filters and ordering.
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.asyncio
+async def test_get_recent_events_returns_logged_rows(
+    patched_session_factory: AsyncSession,
+) -> None:
+    agent_id = await _seed_agent(patched_session_factory)
+    audit = AuditService()
+    task_id = uuid4()
+
+    await audit.log_event(
+        event_type="gateway.rejected",
+        agent_id=agent_id,
+        task_id=task_id,
+        details={"reason": "tracing_gap"},
+    )
+
+    rows = await audit.get_recent_events(limit=10)
+    assert len(rows) >= 1
+    assert any(r["event_type"] == "gateway.rejected" for r in rows)
+
+
+@pytest.mark.asyncio
+async def test_get_recent_events_filters_event_type(
+    patched_session_factory: AsyncSession,
+) -> None:
+    agent_id = await _seed_agent(patched_session_factory)
+    audit = AuditService()
+
+    await audit.log_event(
+        event_type="task.created",
+        agent_id=agent_id,
+        task_id=uuid4(),
+    )
+    await audit.log_event(
+        event_type="task.completed",
+        agent_id=agent_id,
+        task_id=uuid4(),
+    )
+
+    rows = await audit.get_recent_events(limit=10, event_type="task.created")
+    assert all(r["event_type"] == "task.created" for r in rows)
+
+
+@pytest.mark.asyncio
+async def test_get_recent_events_filters_agent_id(
+    patched_session_factory: AsyncSession,
+) -> None:
+    agent_id = await _seed_agent(patched_session_factory)
+    audit = AuditService()
+
+    await audit.log_event(
+        event_type="task.created",
+        agent_id=agent_id,
+        task_id=uuid4(),
+    )
+
+    rows = await audit.get_recent_events(limit=10, agent_id=agent_id)
+    assert all(r["agent_id"] == str(agent_id) for r in rows)
+
+
+@pytest.mark.asyncio
+async def test_get_recent_events_filters_min_severity_warning(
+    patched_session_factory: AsyncSession,
+) -> None:
+    agent_id = await _seed_agent(patched_session_factory)
+    audit = AuditService()
+
+    await audit.log_event(
+        event_type="some.warn",
+        agent_id=agent_id,
+        task_id=uuid4(),
+        severity="warning",
+    )
+    await audit.log_event(
+        event_type="some.info",
+        agent_id=agent_id,
+        task_id=uuid4(),
+        severity="info",
+    )
+
+    rows = await audit.get_recent_events(limit=10, min_severity="warning")
+    assert all(r["severity"] in {"warning", "error"} for r in rows)
+
+
+@pytest.mark.asyncio
+async def test_get_recent_events_filters_min_severity_error(
+    patched_session_factory: AsyncSession,
+) -> None:
+    agent_id = await _seed_agent(patched_session_factory)
+    audit = AuditService()
+
+    await audit.log_event(
+        event_type="error.x",
+        agent_id=agent_id,
+        task_id=uuid4(),
+        severity="error",
+    )
+    await audit.log_event(
+        event_type="warn.x",
+        agent_id=agent_id,
+        task_id=uuid4(),
+        severity="warning",
+    )
+
+    rows = await audit.get_recent_events(limit=10, min_severity="error")
+    assert all(r["severity"] == "error" for r in rows)
+
+
+# ---------------------------------------------------------------------------
+# _resolve_agent_id_by_slug — covers static and DB lookup paths
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.asyncio
+async def test_resolve_agent_id_by_slug_db_lookup(
+    patched_session_factory: AsyncSession,
+) -> None:
+    """An agent NOT in AGENT_UUIDS hits the DB lookup path."""
+    agent = AgentTable(
+        id=uuid4(),
+        name="Runtime Agent",
+        slug=f"runtime-{uuid4().hex[:8]}",
+        role=AgentRole.DEVELOPER,
+        team=None,
+        status=AgentStatus.ACTIVE,
+        model_config={},
+        system_prompt="x",
+        capabilities=[],
+        permissions={},
+        metrics={},
+    )
+    patched_session_factory.add(agent)
+    await patched_session_factory.commit()
+
+    audit = AuditService()
+    resolved = await audit._resolve_agent_id_by_slug(agent.slug)
+    assert resolved == agent.id
+
+
+@pytest.mark.asyncio
+@pytest.mark.usefixtures("patched_session_factory")
+async def test_resolve_agent_id_by_slug_unknown_returns_none() -> None:
+    audit = AuditService()
+    resolved = await audit._resolve_agent_id_by_slug("nonexistent-slug-xyz123")
+    assert resolved is None
+
+
+@pytest.mark.asyncio
+@pytest.mark.usefixtures("patched_session_factory")
+async def test_resolve_agent_id_by_slug_static_lookup_exception(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """Force the AGENT_UUIDS import path to raise to cover lines 465-470."""
+
+    # Replace AGENT_UUIDS with an object that raises on .get
+    class _BadMap:
+        def get(self, *_a: object, **_k: object) -> None:
+            raise RuntimeError("boom")
+
+    monkeypatch.setattr(seeds, "AGENT_UUIDS", _BadMap())
+    audit = AuditService()
+    # Falls back to DB lookup — returns None since slug isn't in DB.
+    resolved = await audit._resolve_agent_id_by_slug("missing-fallback-slug")
+    assert resolved is None
+
+
+@pytest.mark.asyncio
+async def test_resolve_agent_id_by_slug_db_lookup_exception(
+    monkeypatch: pytest.MonkeyPatch,
+) -> None:
+    """Force DB lookup to raise so the outer except runs (lines 488-494)."""
+
+    def _explode() -> object:
+        raise RuntimeError("session factory broken")
+
+    monkeypatch.setattr(db_base, "get_session_factory", _explode)
+    audit = AuditService()
+    resolved = await audit._resolve_agent_id_by_slug("nope-not-real")
+    assert resolved is None

tests/unit/services/test_choreographer_protocol.py

@@ -0,0 +1,64 @@
+"""ChoreographerHelpers stub coverage.
+
+The class is a TYPE_CHECKING-only stub used to give mypy a typed view of the
+Choreographer's helper methods. The real impls live on `_LegacyChoreographer`
+and resolve via MRO — these stubs raise NotImplementedError when called
+directly. We instantiate the bare class and confirm each stub raises so the
+file shows up in coverage.
+"""
+
+from __future__ import annotations
+
+from typing import TYPE_CHECKING
+from uuid import uuid4
+
+import pytest
+from roboco.services.gateway.choreographer._protocol import ChoreographerHelpers
+
+if TYPE_CHECKING:
+    from roboco.services.gateway.envelope import Envelope
+
+
+def _empty_env() -> Envelope:
+    return {
+        "status": "ok",
+        "task_id": None,
+        "next": None,
+        "evidence": None,
+        "context_briefing": None,
+    }
+
+
+@pytest.mark.asyncio
+async def test_emit_rejection_raises() -> None:
+    helpers = ChoreographerHelpers()
+    with pytest.raises(NotImplementedError):
+        await helpers._emit_rejection(
+            _empty_env(), agent_id=uuid4(), task_id=None, verb="x"
+        )
+
+
+@pytest.mark.asyncio
+async def test_briefing_for_raises() -> None:
+    helpers = ChoreographerHelpers()
+    with pytest.raises(NotImplementedError):
+        await helpers._briefing_for(uuid4(), None)
+
+
+def test_with_briefing_raises() -> None:
+    with pytest.raises(NotImplementedError):
+        ChoreographerHelpers._with_briefing(_empty_env(), {})
+
+
+@pytest.mark.asyncio
+async def test_run_claim_guards_raises() -> None:
+    helpers = ChoreographerHelpers()
+    with pytest.raises(NotImplementedError):
+        await helpers._run_claim_guards(agent_id=uuid4(), task=None)
+
+
+@pytest.mark.asyncio
+async def test_touch_raises() -> None:
+    helpers = ChoreographerHelpers()
+    with pytest.raises(NotImplementedError):
+        await helpers._touch(uuid4())

tests/unit/services/test_extraction.py

@@ -2,9 +2,12 @@
 
 from __future__ import annotations
 
+from typing import ClassVar
 from uuid import uuid4
 
+import anthropic as anthropic_mod
 import pytest
+from roboco.llm import ToonAdapter
 from roboco.models import MessageType
 from roboco.models.extraction import (
     ExtractionConfig,
@@ -123,7 +126,8 @@ async def test_extract_splits_on_double_newlines(
 ) -> None:
     content = "First paragraph here.\n\nSecond paragraph here."
     result = await svc.extract(_ctx(content))
-    assert len(result.messages) == 2
+    _PARAS = 2
+    assert len(result.messages) == _PARAS
 
 
 @pytest.mark.asyncio
@@ -145,8 +149,9 @@ async def test_extract_keeps_code_blocks_intact(svc: ExtractionService) -> None:
 async def test_extract_respects_max_segments() -> None:
     svc = ExtractionService(ExtractionConfig(max_segments_per_buffer=2))
     content = "First.\n\nSecond.\n\nThird.\n\nFourth.\n\nFifth."
+    _PARAS = 2
     result = await svc.extract(_ctx(content))
-    assert len(result.messages) <= 2
+    assert len(result.messages) <= _PARAS
 
 
 @pytest.mark.asyncio
@@ -205,10 +210,130 @@ async def test_pipeline_swallows_callback_errors() -> None:
     """Callback failure should not abort the pipeline."""
     pipeline = ExtractionPipeline()
 
-    async def bad_callback(msg) -> None:
+    async def bad_callback(_msg) -> None:
         raise RuntimeError("boom")
 
     pipeline.on_message(bad_callback)
     # Should complete without raising despite callback error.
     result = await pipeline.process_buffer(_ctx("Hello there.\n\nGoodbye."))
     assert result is not None
+
+
+# ---------------------------------------------------------------------------
+# Mentions extraction (line 209)
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.asyncio
+async def test_extract_logs_mentions_found(svc: ExtractionService) -> None:
+    """When @mentions are present, the debug branch fires (line 209)."""
+    content = "@be-dev-1 can you take a look at this code please?"
+    result = await svc.extract(_ctx(content))
+    assert len(result.messages) >= 1
+
+
+@pytest.mark.asyncio
+async def test_extract_skips_empty_segments_from_segmenter(
+    svc: ExtractionService, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """If _segment_content yields a whitespace-only segment, it's skipped.
+
+    Triggers the defensive `if not segment.strip(): continue` (line 193).
+    """
+    monkeypatch.setattr(
+        svc, "_segment_content", lambda _content: ["   ", "Real content here."]
+    )
+    result = await svc.extract(_ctx("Long enough content to bypass min length."))
+    # Only the non-empty segment becomes a message.
+    assert len(result.messages) == 1
+
+
+# ---------------------------------------------------------------------------
+# extract_with_llm (lines 323-404)
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.asyncio
+async def test_extract_with_llm_falls_back_on_error(
+    svc: ExtractionService, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """If the Anthropic call fails, falls back to pattern-based extract."""
+
+    class _BadClient:
+        def __init__(self, **_kwargs: object) -> None:
+            self.messages = self
+
+        async def create(self, **_kwargs: object) -> object:
+            raise RuntimeError("API down")
+
+    monkeypatch.setattr(anthropic_mod, "AsyncAnthropic", _BadClient)
+    result = await svc.extract_with_llm(_ctx("I'm thinking about this."))
+    # Pattern fallback path executes; messages list is not empty.
+    assert result is not None
+    assert len(result.messages) >= 1
+
+
+@pytest.mark.asyncio
+async def test_extract_with_llm_parses_response(
+    svc: ExtractionService, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """Mock a successful LLM response and verify it gets parsed."""
+
+    class _Block:
+        text = "json-or-toon"
+
+    class _Resp:
+        content: ClassVar = [_Block()]
+
+    class _OkClient:
+        def __init__(self, **_kwargs: object) -> None:
+            self.messages = self
+
+        async def create(self, **_kwargs: object) -> _Resp:
+            return _Resp()
+
+    def _decode_dicts(_self: ToonAdapter, _text: str) -> list[dict[str, object]]:
+        return [
+            {"type": "reasoning", "content": "thinking", "confidence": 0.9},
+            {"type": "action", "content": "doing", "confidence": 0.95},
+        ]
+
+    monkeypatch.setattr(anthropic_mod, "AsyncAnthropic", _OkClient)
+    monkeypatch.setattr(ToonAdapter, "decode", _decode_dicts)
+
+    result = await svc.extract_with_llm(_ctx("Some content for the LLM."))
+    assert result is not None
+    _COUNT = 2
+    assert len(result.messages) == _COUNT
+
+
+@pytest.mark.asyncio
+async def test_extract_with_llm_handles_non_dict_segments(
+    svc: ExtractionService, monkeypatch: pytest.MonkeyPatch
+) -> None:
+    """When the toon decoder returns non-dict segments, fallback type."""
+
+    class _Block:
+        text = "raw response"
+
+    class _Resp:
+        content: ClassVar = [_Block()]
+
+    class _OkClient:
+        def __init__(self, **_kwargs: object) -> None:
+            self.messages = self
+
+        async def create(self, **_kwargs: object) -> _Resp:
+            return _Resp()
+
+    # Patch toon to yield strings instead of dicts.
+    def _decode_strings(_self: ToonAdapter, _text: str) -> list[str]:
+        return ["plain string segment", "another"]
+
+    monkeypatch.setattr(anthropic_mod, "AsyncAnthropic", _OkClient)
+    monkeypatch.setattr(ToonAdapter, "decode", _decode_strings)
+
+    result = await svc.extract_with_llm(_ctx("Some content for the LLM."))
+    assert result is not None
+    _COUNT = 2
+    assert len(result.messages) == _COUNT

tests/unit/services/test_git_commit_trailer.py

@@ -4,7 +4,11 @@ import importlib
 
 import pytest
 import roboco.config as config_module
-from roboco.templates.git.commit import CommitContext, build_commit_message
+from roboco.templates.git.commit import (
+    CommitContext,
+    CommitMessageError,
+    build_commit_message,
+)
 
 
 def _make_ctx() -> CommitContext:
@@ -48,3 +52,89 @@ def test_links_use_public_base_url(monkeypatch: pytest.MonkeyPatch) -> None:
         assert "127.0.0.1" not in out
     finally:
         importlib.reload(config_module)
+
+
+def test_commit_context_invalid_type_raises() -> None:
+    """Lines 49-52: invalid commit_type raises CommitMessageError."""
+    with pytest.raises(CommitMessageError, match="Invalid commit type"):
+        CommitContext(
+            task_id="t-1",
+            root_task_id="r-1",
+            agent_slug="be-dev-1",
+            session_id="s-1",
+            commit_type="garbage",
+            scope=None,
+            description="x",
+        )
+
+
+def test_commit_context_missing_description_raises() -> None:
+    """Line 54: missing description raises."""
+    with pytest.raises(CommitMessageError, match="description is required"):
+        CommitContext(
+            task_id="t-1",
+            root_task_id="r-1",
+            agent_slug="be-dev-1",
+            session_id="s-1",
+            commit_type="feat",
+            scope=None,
+            description="",
+        )
+
+
+def test_commit_context_missing_task_id_raises() -> None:
+    """Line 56: missing task_id raises."""
+    with pytest.raises(CommitMessageError, match="Task ID is required"):
+        CommitContext(
+            task_id="",
+            root_task_id="r-1",
+            agent_slug="be-dev-1",
+            session_id="s-1",
+            commit_type="feat",
+            scope=None,
+            description="x",
+        )
+
+
+def test_commit_context_missing_root_task_id_raises() -> None:
+    """Line 58: missing root_task_id raises."""
+    with pytest.raises(CommitMessageError, match="Root task ID is required"):
+        CommitContext(
+            task_id="t-1",
+            root_task_id="",
+            agent_slug="be-dev-1",
+            session_id="s-1",
+            commit_type="feat",
+            scope=None,
+            description="x",
+        )
+
+
+def test_commit_context_missing_agent_slug_raises() -> None:
+    """Line 60: missing agent_slug raises."""
+    with pytest.raises(CommitMessageError, match="Agent slug is required"):
+        CommitContext(
+            task_id="t-1",
+            root_task_id="r-1",
+            agent_slug="",
+            session_id="s-1",
+            commit_type="feat",
+            scope=None,
+            description="x",
+        )
+
+
+def test_build_commit_message_with_body() -> None:
+    """Line 85: ctx.body present → body section appended."""
+    ctx = CommitContext(
+        task_id="t-1",
+        root_task_id="r-1",
+        agent_slug="be-dev-1",
+        session_id="s-1",
+        commit_type="feat",
+        scope="auth",
+        description="add login",
+        body="Implements OAuth2 login flow",
+    )
+    out = build_commit_message(ctx, "https://example.com/api")
+    assert "Implements OAuth2 login flow" in out

tests/unit/services/test_health.py

@@ -0,0 +1,85 @@
+"""roboco.services.health coverage — DB / Redis connectivity probes.
+
+Both probes are thin wrappers: they execute a trivial command and return
+("ok", True) on success, (str(error), False) on failure. We mock the
+underlying clients so the test doesn't depend on Redis/Postgres uptime.
+"""
+
+from __future__ import annotations
+
+from unittest.mock import AsyncMock, MagicMock, patch
+
+import pytest
+from roboco.services.health import check_database, check_redis
+
+# ---------------------------------------------------------------------------
+# check_database
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.asyncio
+async def test_check_database_ok() -> None:
+    """Successful SELECT 1 returns ('ok', True)."""
+    mock_session = MagicMock()
+    mock_session.execute = AsyncMock()
+
+    class _Ctx:
+        async def __aenter__(self) -> object:
+            return mock_session
+
+        async def __aexit__(self, *_args: object) -> None:
+            return None
+
+    with patch("roboco.services.health.get_db_context", return_value=_Ctx()):
+        msg, ok = await check_database()
+    assert ok is True
+    assert msg == "ok"
+
+
+@pytest.mark.asyncio
+async def test_check_database_error() -> None:
+    """Any exception inside the context is captured into ('<msg>', False)."""
+
+    class _BadCtx:
+        async def __aenter__(self) -> object:
+            raise RuntimeError("connection refused")
+
+        async def __aexit__(self, *_args: object) -> None:
+            return None
+
+    with patch("roboco.services.health.get_db_context", return_value=_BadCtx()):
+        msg, ok = await check_database()
+    assert ok is False
+    assert "connection refused" in msg
+
+
+# ---------------------------------------------------------------------------
+# check_redis
+# ---------------------------------------------------------------------------
+
+
+@pytest.mark.asyncio
+async def test_check_redis_ok() -> None:
+    """Successful ping returns ('ok', True)."""
+    mock_client = MagicMock()
+    mock_client.ping = AsyncMock(return_value=True)
+    mock_client.close = AsyncMock()
+
+    with patch("roboco.services.health.redis.from_url", return_value=mock_client):
+        msg, ok = await check_redis()
+    assert ok is True
+    assert msg == "ok"
+    mock_client.ping.assert_awaited_once()
+    mock_client.close.assert_awaited_once()
+
+
+@pytest.mark.asyncio
+async def test_check_redis_error() -> None:
+    """Connection failure is caught and reported as a tuple."""
+    mock_client = MagicMock()
+    mock_client.ping = AsyncMock(side_effect=ConnectionError("redis down"))
+
+    with patch("roboco.services.health.redis.from_url", return_value=mock_client):
+        msg, ok = await check_redis()
+    assert ok is False
+    assert "redis down" in msg

tests/unit/services/test_learning.py

@@ -20,6 +20,8 @@ from roboco.services.learning import (
     LearningScope,
     LearningType,
     RecordLearningParams,
+    _LearningServiceHolder,
+    get_learning_service,
 )
 
 
@@ -112,7 +114,7 @@ async def test_record_learning_normalizes_string_enums(
 async def test_record_learning_team_scope_calls_create_notifications(
     svc: LearningPropagationService,
 ) -> None:
-    """Team-scope learnings call _create_notifications, which best-effort logs on error."""
+    """Team-scope learnings call _create_notifications; best-effort logs on error."""
     stub = _StubOptimal()
     await svc.initialize(stub)
     # The notifications branch will silently fail because there's no DB
@@ -180,7 +182,8 @@ async def test_get_learnings_for_agent_returns_filtered_results(
     )
     await svc.initialize(stub)
     out = await svc.get_learnings_for_agent(aid, "developer")
-    # Visible: own personal, team for matching role, org-anyone — drop other-personal & team-other-role
+    # Visible: own personal, team for matching role, org-anyone.
+    # Filtered out: other-personal & team-other-role.
     contents = [r.metadata for r in out]
     assert own_personal.metadata in contents
     assert team_visible.metadata in contents
@@ -215,7 +218,8 @@ async def test_search_similar_learnings_passes_through(
     await svc.initialize(stub)
     out = await svc.search_similar_learnings("how to debug", top_k=3)
     assert len(out) == 1
-    assert stub.searches[0]["top_k"] == 3
+    _TOP_K = 3
+    assert stub.searches[0]["top_k"] == _TOP_K
     assert IndexType.LEARNINGS in stub.searches[0]["index_types"]
 
 
@@ -318,3 +322,13 @@ async def test_get_learning_stats_returns_dict_with_expected_keys(
     assert "total_learnings" in stats
     assert "by_type" in stats
     assert "by_scope" in stats
+
+
+@pytest.mark.asyncio
+async def test_get_learning_service_factory() -> None:
+    """get_learning_service returns a singleton instance."""
+    _LearningServiceHolder.instance = None
+    a = await get_learning_service()
+    b = await get_learning_service()
+    assert a is b
+    _LearningServiceHolder.instance = None

tests/unit/services/test_notification.py

@@ -14,6 +14,7 @@ from uuid import UUID, uuid4
 
 import pytest
 from roboco.models import NotificationPriority, NotificationType
+from roboco.models.notification import CreateNotificationParams
 from roboco.services.notification import (
     NotificationService,
     _resolve_agent_uuid,
@@ -232,8 +233,6 @@ async def test_create_notification_skips_when_from_agent_unresolvable(
 ) -> None:
     """Unresolvable from_agent → log and skip, no row inserted."""
     db = _FakeDb(agent_uuid=None)  # All slug lookups return None.
-    from roboco.models.notification import CreateNotificationParams
-
     with _patch_db_context(db):
         await svc._create_notification(
             CreateNotificationParams(
@@ -254,7 +253,6 @@ async def test_create_notification_skips_when_no_resolvable_recipients(
 ) -> None:
     """All recipients unresolvable → skip with warn."""
     aid = uuid4()
-    from roboco.models.notification import CreateNotificationParams
 
     # First call resolves from_agent, subsequent slug lookups still hit our
     # fake — which always returns the same agent. Use a fake that returns the

tests/unit/services/test_permissions.py

@@ -7,6 +7,7 @@ The service is a SingletonService, so we instantiate it directly with
 
 from __future__ import annotations
 
+from unittest.mock import patch
 from uuid import uuid4
 
 import pytest
@@ -22,8 +23,8 @@ from roboco.services.permissions import PermissionService
 
 @pytest.fixture
 def svc() -> PermissionService:
-    """PermissionService is a SingletonService — bypass __init__ for unit tests."""
-    return object.__new__(PermissionService)
+    """PermissionService is a SingletonService — call __init__ to bind log."""
+    return PermissionService()
 
 
 def _ctx(role: AgentRole, team: Team | None = None) -> AgentContext:
@@ -338,3 +339,101 @@ def test_can_agent_write_channel_unknown_channel(
 ) -> None:
     """Unknown channel slug → False (no panic)."""
     assert svc.can_agent_write_channel("be-dev-1", "ghost-channel") is False
+
+
+# ---------------------------------------------------------------------------
+# Channel read for non-bypass roles (covers _check_channel_access_for_agent)
+# ---------------------------------------------------------------------------
+
+
+def test_dev_read_unknown_channel_returns_false(svc: PermissionService) -> None:
+    """Unknown channel for non-bypass role → warns + returns False (lines 137-138)."""
+    dev = _ctx(AgentRole.DEVELOPER, team=Team.BACKEND)
+    assert svc.can_read_channel(dev, "ghost-channel-x") is False
+
+
+def test_dev_write_unknown_channel_returns_false(svc: PermissionService) -> None:
+    """Unknown channel for non-bypass role on write → False."""
+    dev = _ctx(AgentRole.DEVELOPER, team=Team.BACKEND)
+    assert svc.can_write_channel(dev, "ghost-channel-y") is False
+
+
+def test_dev_can_read_own_cell_channel(svc: PermissionService) -> None:
+    """Developer in backend can read backend-cell (regular role-based access)."""
+    dev = _ctx(AgentRole.DEVELOPER, team=Team.BACKEND)
+    assert svc.can_read_channel(dev, "backend-cell") is True
+
+
+# ---------------------------------------------------------------------------
+# can_notify branches
+# ---------------------------------------------------------------------------
+
+
+def test_can_notify_developer_returns_false(svc: PermissionService) -> None:
+    """Developers cannot send notifications — short-circuits on line 236."""
+    dev = _ctx(AgentRole.DEVELOPER, team=Team.BACKEND)
+    other = _ctx(AgentRole.QA, team=Team.BACKEND)
+    assert svc.can_notify(dev, other) is False
+
+
+# ---------------------------------------------------------------------------
+# can_agent_read_channel for unknown channel (line 377)
+# ---------------------------------------------------------------------------
+
+
+def test_can_agent_read_channel_unknown_channel(svc: PermissionService) -> None:
+    """Channel not in CHANNEL_ACCESS → False (line 377)."""
+    assert svc.can_agent_read_channel("be-dev-1", "ghost-channel-z") is False
+
+
+# ---------------------------------------------------------------------------
+# can_notify list scope (lines 253-258)
+# ---------------------------------------------------------------------------
+
+
+def test_can_notify_product_owner_list_scope_in(svc: PermissionService) -> None:
+    """Product Owner has list scope — allowed recipients return True."""
+    sender = _ctx(AgentRole.PRODUCT_OWNER, team=Team.BOARD)
+    recipient = _ctx(AgentRole.MAIN_PM, team=Team.MAIN_PM)
+    assert svc.can_notify(sender, recipient) is True
+
+
+def test_can_notify_product_owner_list_scope_out(svc: PermissionService) -> None:
+    """Product Owner cannot notify recipients outside their list scope."""
+    sender = _ctx(AgentRole.PRODUCT_OWNER, team=Team.BOARD)
+    recipient = _ctx(AgentRole.DEVELOPER, team=Team.BACKEND)
+    assert svc.can_notify(sender, recipient) is False
+
+
+# ---------------------------------------------------------------------------
+# can_perform_task_action VIEW_ALL fallback for VIEW_OWN (line 310)
+# ---------------------------------------------------------------------------
+
+
+def test_view_own_falls_back_to_view_all_for_ceo(svc: PermissionService) -> None:
+    """CEO has VIEW_ALL but not VIEW_OWN — VIEW_OWN check falls back to True."""
+    ceo = _ctx(AgentRole.CEO)
+    assert svc.can_perform_task_action(ceo, TaskAction.VIEW_OWN, Team.BACKEND) is True
+
+
+def test_check_channel_access_silent_observer_grants_read(
+    svc: PermissionService,
+) -> None:
+    """Line 151: agent slug in silent list grants read access via direct call."""
+    auditor = _ctx(AgentRole.AUDITOR, team=Team.BOARD)
+    # _check_channel_access_for_agent bypasses the auditor short-circuit at
+    # can_read_channel and exercises the silent-list match (line 150-151).
+    assert svc._check_channel_access_for_agent(auditor, "backend-cell", "read") is True
+
+
+def test_can_notify_unknown_scope_returns_false(svc: PermissionService) -> None:
+    """Line 258: scope is neither 'all', 'cell', nor list → defensive return False."""
+
+    sender = _ctx(AgentRole.MAIN_PM, team=Team.MAIN_PM)
+    recipient = _ctx(AgentRole.DEVELOPER, team=Team.BACKEND)
+    # Patch _get_notification_scope directly to a bogus value type.
+    with patch(
+        "roboco.services.permissions._get_notification_scope",
+        return_value="garbage_scope",
+    ):
+        assert svc.can_notify(sender, recipient) is False

tests/unit/services/test_services_base.py

@@ -0,0 +1,133 @@
+"""Coverage for roboco.services.base — SingletonHolder pattern."""
+
+from __future__ import annotations
+
+import pytest
+from roboco.services.base import (
+    BaseService,
+    ConflictError,
+    NotFoundError,
+    ServiceError,
+    ServiceUnavailableError,
+    SingletonHolder,
+    SingletonService,
+    UnauthorizedError,
+    ValidationError,
+)
+
+
+class _DummyService:
+    def __init__(self, value: str = "default") -> None:
+        self.value = value
+
+
+class _Holder(SingletonHolder[_DummyService]):
+    def create_instance(self) -> _DummyService:
+        return _DummyService("first")
+
+
+def test_singleton_holder_get_creates_instance() -> None:
+    holder = _Holder()
+    assert holder.is_initialized is False
+    inst = holder.get()
+    assert inst.value == "first"
+    assert holder.is_initialized is True
+
+
+def test_singleton_holder_get_returns_same_instance() -> None:
+    holder = _Holder()
+    a = holder.get()
+    b = holder.get()
+    assert a is b
+
+
+def test_singleton_holder_set_overrides_instance() -> None:
+    holder = _Holder()
+    custom = _DummyService("custom")
+    holder.set(custom)
+    assert holder.get() is custom
+
+
+def test_singleton_holder_clear_resets_instance() -> None:
+    holder = _Holder()
+    holder.get()
+    holder.clear()
+    assert holder.is_initialized is False
+    # Subsequent get() rebuilds.
+    new = holder.get()
+    assert new.value == "first"
+
+
+def test_singleton_holder_create_instance_default_raises() -> None:
+    """Base SingletonHolder must require subclass override."""
+    holder: SingletonHolder[_DummyService] = SingletonHolder()
+    with pytest.raises(NotImplementedError):
+        holder.get()
+
+
+# ---------------------------------------------------------------------------
+# Error hierarchy — round-trip the constructors so the dataclass-style fields
+# get exercised end-to-end.
+# ---------------------------------------------------------------------------
+
+
+def test_service_error_basic() -> None:
+    err = ServiceError("oops", details={"k": "v"})
+    assert err.message == "oops"
+    assert err.details == {"k": "v"}
+
+
+def test_not_found_error_with_id() -> None:
+    err = NotFoundError("Task", resource_id="abc")
+    assert "abc" in err.message
+    assert err.resource_id == "abc"
+
+
+def test_not_found_error_without_id() -> None:
+    err = NotFoundError("Task")
+    assert err.message == "Task not found"
+
+
+def test_validation_error_with_field() -> None:
+    err = ValidationError("bad", field="title")
+    assert err.field == "title"
+
+
+def test_conflict_error_records_resource() -> None:
+    err = ConflictError("dup", resource_type="Project")
+    assert err.resource_type == "Project"
+
+
+def test_unauthorized_error_with_reason() -> None:
+    err = UnauthorizedError("delete", reason="role")
+    assert "delete" in err.message
+    assert "role" in err.message
+
+
+def test_service_unavailable_error_with_reason() -> None:
+    err = ServiceUnavailableError("postgres", reason="down")
+    assert "postgres" in err.message
+    assert "down" in err.message
+
+
+# ---------------------------------------------------------------------------
+# BaseService + SingletonService instantiation — bind logger, etc.
+# ---------------------------------------------------------------------------
+
+
+def test_base_service_binds_session_and_logger() -> None:
+    class _Svc(BaseService):
+        service_name = "x"
+
+    fake_session = object()
+    svc = _Svc(fake_session)  # type: ignore[arg-type]
+    assert svc.session is fake_session
+    assert svc.log is not None
+
+
+def test_singleton_service_binds_logger() -> None:
+    class _Svc(SingletonService):
+        service_name = "y"
+
+    svc = _Svc()
+    assert svc.log is not None