diff --git a/roboco/api/routes/playbooks.py b/roboco/api/routes/playbooks.py index eb99eee6..50cfe78e 100644 --- a/roboco/api/routes/playbooks.py +++ b/roboco/api/routes/playbooks.py @@ -53,14 +53,17 @@ async def approve_playbook( """Approve a draft playbook → approved (and indexed into the KB).""" _require_curator(agent) try: - playbook = await get_playbook_service(db).approve( - playbook_id, approver_id=agent.agent_id - ) + svc = get_playbook_service(db) + playbook = await svc.approve(playbook_id, approver_id=agent.agent_id) except NotFoundError as exc: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, detail="Playbook not found" ) from exc + # Commit the status change BEFORE indexing: the RAG index write runs through + # its own auto-committing connection, so indexing before commit would durably + # land an approved playbook in the corpus even if this commit rolled back. await db.commit() + await svc.index_approved(playbook) return Playbook.model_validate(playbook) @@ -74,12 +77,15 @@ async def reject_playbook( """Reject a playbook → archived, with the Auditor's reason.""" _require_curator(agent) try: - playbook = await get_playbook_service(db).reject( + svc = get_playbook_service(db) + playbook = await svc.reject( playbook_id, approver_id=agent.agent_id, reason=body.reason ) except NotFoundError as exc: raise HTTPException( status_code=status.HTTP_404_NOT_FOUND, detail="Playbook not found" ) from exc + # Commit the status change BEFORE de-indexing (see approve_playbook). await db.commit() + await svc.unindex_playbook(playbook) return Playbook.model_validate(playbook) diff --git a/roboco/services/gateway/content_actions.py b/roboco/services/gateway/content_actions.py index 1e248870..e9ea4a1a 100644 --- a/roboco/services/gateway/content_actions.py +++ b/roboco/services/gateway/content_actions.py @@ -793,6 +793,18 @@ class ContentActions: status = "playbook_archived" except NotFoundError: return Envelope.not_found(message=f"playbook {playbook_id} not found") + # Commit the status change BEFORE touching the RAG index: the index write + # runs through its own auto-committing connection, so indexing before the + # status commit would durably land (or drop) a playbook in the corpus even + # if this transaction rolled back — a divergence agents surface in + # briefings. ``get_db`` commits the session again after the route returns + # (a no-op on the now-clean transaction); this explicit commit is what + # gates the index. + await self.task.session.commit() + if action == "approve": + await svc.index_approved(playbook) + else: + await svc.unindex_playbook(playbook) return Envelope.ok( status=status, task_id=None, diff --git a/roboco/services/playbook.py b/roboco/services/playbook.py index b402aa92..4c472fba 100644 --- a/roboco/services/playbook.py +++ b/roboco/services/playbook.py @@ -68,21 +68,30 @@ class PlaybookService(BaseService): return playbook async def approve(self, playbook_id: UUID, approver_id: UUID) -> PlaybookTable: - """Auditor approves a draft: draft -> approved, stamped.""" + """Auditor approves a draft: draft -> approved, stamped. + + Flushes the status change ONLY — the RAG index write (``index_approved``) + is a SEPARATE step the caller runs AFTER committing the status. The vector + store writes through its own auto-committing pool connection, so indexing + inline (before the status commit) would durably land an approved playbook + in the corpus even if the status transaction rolled back — a divergence + agents then surfaced in briefings. + """ playbook = await self._get_or_raise(playbook_id) playbook.status = PlaybookStatus.APPROVED.value playbook.approved_by = approver_id playbook.approved_at = datetime.now(UTC) await self.session.flush() - await self._index_approved(playbook) self.log.info("Playbook approved", playbook_id=str(playbook_id)) return playbook - async def _index_approved(self, playbook: PlaybookTable) -> None: + async def index_approved(self, playbook: PlaybookTable) -> None: """Embed an approved playbook into the PLAYBOOKS RAG index (best-effort). - Gated on ``org_memory_enabled`` so the feature is fully inert when off; - a failure (e.g. the embedder is down) never blocks the approval. + Post-commit step: the caller commits the ``draft -> approved`` status + change FIRST, then runs this so the index never leads the status + transaction. Gated on ``org_memory_enabled`` so the feature is fully inert + when off; a failure (e.g. the embedder is down) never blocks the approval. """ if not settings.org_memory_enabled: return @@ -114,17 +123,20 @@ class PlaybookService(BaseService): async def reject( self, playbook_id: UUID, approver_id: UUID, reason: str ) -> PlaybookTable: - """Auditor rejects a playbook: -> archived (reason recorded in the log).""" + """Auditor rejects a playbook: -> archived (reason recorded in the log). + + Flushes the status change ONLY — ``unindex_playbook`` is a separate + post-commit step (see ``approve`` for the ordering rationale). + """ playbook = await self._get_or_raise(playbook_id) playbook.status = PlaybookStatus.ARCHIVED.value playbook.approved_by = approver_id playbook.approved_at = datetime.now(UTC) await self.session.flush() - await self._unindex_playbook(playbook) self.log.info("Playbook rejected", playbook_id=str(playbook_id), reason=reason) return playbook - async def _unindex_playbook(self, playbook: PlaybookTable) -> None: + async def unindex_playbook(self, playbook: PlaybookTable) -> None: """De-index a playbook from the PLAYBOOKS RAG index (best-effort). The mirror of :meth:`_index_approved`: a rejected/archived playbook that diff --git a/tests/unit/gateway/test_playbook_verbs.py b/tests/unit/gateway/test_playbook_verbs.py index d9efd0da..5e471d0f 100644 --- a/tests/unit/gateway/test_playbook_verbs.py +++ b/tests/unit/gateway/test_playbook_verbs.py @@ -48,7 +48,7 @@ def _actions(role: str) -> ContentActions: agent = MagicMock() agent.role = role task.agent_for = AsyncMock(return_value=agent) - task.session = MagicMock() + task.session = AsyncMock() deps = ContentActionsDeps( task=task, git=MagicMock(), @@ -107,13 +107,17 @@ async def test_approve_playbook_for_auditor(monkeypatch: pytest.MonkeyPatch) -> approved.status = "approved" svc = MagicMock() svc.approve = AsyncMock(return_value=approved) + svc.index_approved = AsyncMock() monkeypatch.setattr("roboco.services.playbook.get_playbook_service", lambda _s: svc) - env = await _actions("auditor").approve_playbook( - agent_id=uuid4(), playbook_id=uuid4() - ) + actions = _actions("auditor") + env = await actions.approve_playbook(agent_id=uuid4(), playbook_id=uuid4()) assert env.error is None assert env.status == "playbook_approved" svc.approve.assert_awaited_once() + # F057: the status commit gates the index — commit then index, never index + # before commit (the index write auto-commits on its own connection). + actions.task.session.commit.assert_awaited_once() + svc.index_approved.assert_awaited_once_with(approved) @pytest.mark.asyncio @@ -125,9 +129,14 @@ async def test_reject_playbook_archives_for_auditor( archived.status = "archived" svc = MagicMock() svc.reject = AsyncMock(return_value=archived) + svc.unindex_playbook = AsyncMock() monkeypatch.setattr("roboco.services.playbook.get_playbook_service", lambda _s: svc) - env = await _actions("auditor").reject_playbook( + actions = _actions("auditor") + env = await actions.reject_playbook( agent_id=uuid4(), playbook_id=uuid4(), reason="duplicate" ) assert env.status == "playbook_archived" svc.reject.assert_awaited_once() + # F057: de-index is the post-commit step (commit gates it). + actions.task.session.commit.assert_awaited_once() + svc.unindex_playbook.assert_awaited_once_with(archived) diff --git a/tests/unit/services/test_playbook_index_ordering.py b/tests/unit/services/test_playbook_index_ordering.py new file mode 100644 index 00000000..39b49e6f --- /dev/null +++ b/tests/unit/services/test_playbook_index_ordering.py @@ -0,0 +1,335 @@ +"""F057: the PLAYBOOKS RAG index write must not commit independently of — and +BEFORE — the playbook status transaction. + +``approve()`` / ``reject()`` used to call ``_index_approved`` / ``_unindex_playbook`` +inline, AFTER ``flush()`` but BEFORE the caller's ``commit()``. The vector store +writes chunks via its OWN pool connection (vector_store.py:211-237), which +auto-commits immediately and independently of the SQLAlchemy session +transaction. So a status-commit failure (or a crash between the index write and +the commit) left the RAG corpus with an approved/archived playbook whose DB row +was still DRAFT/APPROVED — a divergence agents then surfaced in briefings. + +The fix: ``approve()`` / ``reject()`` flush the status ONLY; the index/unindex +is a separate post-commit step (``index_approved`` / ``unindex_playbook``) the +caller runs AFTER the status transaction commits. Both entry points — the panel +route (playbooks.py) and the Auditor gateway verb (content_actions. +_curate_playbook) — commit-then-index, and skip the index if the commit fails. +""" + +from __future__ import annotations + +from typing import Any +from unittest.mock import AsyncMock, MagicMock +from uuid import uuid4 + +import pytest +from roboco.api.routes.playbooks import approve_playbook, reject_playbook +from roboco.config import settings +from roboco.models import AgentRole +from roboco.models.permissions import AgentContext +from roboco.services.gateway.content_actions import ContentActions, ContentActionsDeps +from roboco.services.playbook import PlaybookService + +_PID = uuid4() +_APPROVER = uuid4() + + +def _playbook_mock() -> Any: + pb = MagicMock(name="playbook") + pb.id = _PID + pb.status = "approved" + pb.title = "T" + pb.problem = "P" + pb.procedure = "Pr" + pb.tags = [] + pb.team = "backend" + pb.scope = "team" + return pb + + +# --------------------------------------------------------------------------- +# Service-level: approve/reject flush ONLY; index/unindex are separate steps +# --------------------------------------------------------------------------- + + +@pytest.mark.asyncio +async def test_approve_does_not_index_before_commit( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """``approve()`` flushes the status change but must NOT write to the RAG + index — that writes through its own auto-committing connection, so it would + durably land before the caller commits the status (the F057 divergence).""" + monkeypatch.setattr(settings, "org_memory_enabled", True) + session = AsyncMock() + session.flush = AsyncMock() + svc = PlaybookService(session) + svc._get_or_raise = AsyncMock(return_value=_playbook_mock()) # type: ignore[assignment] + + optimal = MagicMock() + optimal.index_playbook = AsyncMock() + monkeypatch.setattr( + "roboco.services.optimal.get_optimal_service", + AsyncMock(return_value=optimal), + ) + + await svc.approve(_PID, approver_id=_APPROVER) + + optimal.index_playbook.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_index_approved_is_a_separate_post_commit_step( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """``index_approved`` is the public post-commit index step the caller runs + AFTER committing the status — it invokes the optimal index.""" + monkeypatch.setattr(settings, "org_memory_enabled", True) + svc = PlaybookService(AsyncMock()) + optimal = MagicMock() + optimal.index_playbook = AsyncMock() + monkeypatch.setattr( + "roboco.services.optimal.get_optimal_service", + AsyncMock(return_value=optimal), + ) + + await svc.index_approved(_playbook_mock()) + + optimal.index_playbook.assert_awaited_once() + + +@pytest.mark.asyncio +async def test_reject_does_not_unindex_before_commit( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """``reject()`` flushes the status change but must NOT de-index from the RAG + index inline — same ordering gap as approve (the de-index auto-commits before + the status commit, dropping an approved playbook from briefings while its row + is still APPROVED on rollback).""" + monkeypatch.setattr(settings, "org_memory_enabled", True) + session = AsyncMock() + session.flush = AsyncMock() + svc = PlaybookService(session) + svc._get_or_raise = AsyncMock(return_value=_playbook_mock()) # type: ignore[assignment] + + optimal = MagicMock() + optimal.unindex_playbook = AsyncMock() + optimal.deindex_playbook = AsyncMock() + monkeypatch.setattr( + "roboco.services.optimal.get_optimal_service", + AsyncMock(return_value=optimal), + ) + + await svc.reject(_PID, approver_id=_APPROVER, reason="nope") + + # No de-index call of any name originated from reject() before commit. + optimal.unindex_playbook.assert_not_awaited() + optimal.deindex_playbook.assert_not_awaited() + + +@pytest.mark.asyncio +async def test_unindex_playbook_is_a_separate_post_commit_step( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """``unindex_playbook`` is the public post-commit de-index step.""" + monkeypatch.setattr(settings, "org_memory_enabled", True) + svc = PlaybookService(AsyncMock()) + optimal = MagicMock() + optimal.unindex_playbook = AsyncMock() + monkeypatch.setattr( + "roboco.services.optimal.get_optimal_service", + AsyncMock(return_value=optimal), + ) + + await svc.unindex_playbook(_playbook_mock()) + + optimal.unindex_playbook.assert_awaited_once() + + +# --------------------------------------------------------------------------- +# Route-level (panel): commit-then-index, skip index on commit failure +# --------------------------------------------------------------------------- + + +def _route_service_mock( + order: list[str], *, commit_fails: bool = False +) -> tuple[Any, Any]: + def _approve(*_a: Any, **_k: Any) -> Any: + order.append("approve") + return _playbook_mock() + + def _reject(*_a: Any, **_k: Any) -> Any: + order.append("reject") + return _playbook_mock() + + svc = MagicMock() + svc.approve = AsyncMock(side_effect=_approve) + svc.reject = AsyncMock(side_effect=_reject) + svc.index_approved = AsyncMock(side_effect=lambda *_a, **_k: order.append("index")) + svc.unindex_playbook = AsyncMock( + side_effect=lambda *_a, **_k: order.append("unindex") + ) + db = AsyncMock() + if commit_fails: + db.commit = AsyncMock(side_effect=RuntimeError("commit failed")) + else: + db.commit = AsyncMock(side_effect=lambda: order.append("commit")) + return svc, db + + +@pytest.mark.asyncio +async def test_approve_route_commits_before_indexing( + monkeypatch: pytest.MonkeyPatch, +) -> None: + order: list[str] = [] + svc, db = _route_service_mock(order) + monkeypatch.setattr( + "roboco.api.routes.playbooks.get_playbook_service", lambda _db: svc + ) + monkeypatch.setattr( + "roboco.api.routes.playbooks.Playbook.model_validate", lambda obj: obj + ) + agent = AgentContext(agent_id=_APPROVER, role=AgentRole.AUDITOR) + + await approve_playbook(_PID, db, agent) + + assert order == ["approve", "commit", "index"] + + +@pytest.mark.asyncio +async def test_approve_route_skips_index_when_commit_fails( + monkeypatch: pytest.MonkeyPatch, +) -> None: + order: list[str] = [] + svc, db = _route_service_mock(order, commit_fails=True) + monkeypatch.setattr( + "roboco.api.routes.playbooks.get_playbook_service", lambda _db: svc + ) + agent = AgentContext(agent_id=_APPROVER, role=AgentRole.AUDITOR) + + with pytest.raises(RuntimeError): + await approve_playbook(_PID, db, agent) + + svc.index_approved.assert_not_awaited() + assert order == ["approve"] + + +@pytest.mark.asyncio +async def test_reject_route_commits_before_unindexing( + monkeypatch: pytest.MonkeyPatch, +) -> None: + order: list[str] = [] + svc, db = _route_service_mock(order) + monkeypatch.setattr( + "roboco.api.routes.playbooks.get_playbook_service", lambda _db: svc + ) + monkeypatch.setattr( + "roboco.api.routes.playbooks.Playbook.model_validate", lambda obj: obj + ) + agent = AgentContext(agent_id=_APPROVER, role=AgentRole.AUDITOR) + + await reject_playbook(_PID, MagicMock(reason="nope"), db, agent) + + assert order == ["reject", "commit", "unindex"] + + +# --------------------------------------------------------------------------- +# Gateway-level (Auditor verb): commit-then-index, skip index on commit failure +# --------------------------------------------------------------------------- + + +def _gateway_actions( + order: list[str], *, commit_fails: bool = False +) -> tuple[ContentActions, MagicMock]: + task = MagicMock() + task.session = AsyncMock() + if commit_fails: + task.session.commit = AsyncMock(side_effect=RuntimeError("commit failed")) + else: + task.session.commit = AsyncMock(side_effect=lambda: order.append("commit")) + task.agent_for = AsyncMock(return_value=MagicMock(role="auditor")) + deps = ContentActionsDeps( + task=task, + git=MagicMock(), + messaging=MagicMock(), + a2a=MagicMock(), + journal=MagicMock(), + workspace=MagicMock(), + notifications=MagicMock(), + ) + return ContentActions(deps), task + + +def _gateway_svc_mock(order: list[str]) -> MagicMock: + def _approve(*_a: Any, **_k: Any) -> Any: + order.append("approve") + return _playbook_mock() + + def _reject(*_a: Any, **_k: Any) -> Any: + order.append("reject") + return _playbook_mock() + + svc = MagicMock() + svc.approve = AsyncMock(side_effect=_approve) + svc.reject = AsyncMock(side_effect=_reject) + svc.index_approved = AsyncMock(side_effect=lambda *_a, **_k: order.append("index")) + svc.unindex_playbook = AsyncMock( + side_effect=lambda *_a, **_k: order.append("unindex") + ) + return svc + + +@pytest.mark.asyncio +async def test_gateway_approve_commits_before_indexing( + monkeypatch: pytest.MonkeyPatch, +) -> None: + order: list[str] = [] + actions, _task = _gateway_actions(order) + svc = _gateway_svc_mock(order) + monkeypatch.setattr( + "roboco.services.playbook.get_playbook_service", + lambda _session: svc, + ) + + env = await actions.approve_playbook(agent_id=_APPROVER, playbook_id=_PID) + + assert order == ["approve", "commit", "index"] + assert env.status == "playbook_approved" + + +@pytest.mark.asyncio +async def test_gateway_approve_skips_index_when_commit_fails( + monkeypatch: pytest.MonkeyPatch, +) -> None: + order: list[str] = [] + actions, _task = _gateway_actions(order, commit_fails=True) + svc = _gateway_svc_mock(order) + monkeypatch.setattr( + "roboco.services.playbook.get_playbook_service", + lambda _session: svc, + ) + + with pytest.raises(RuntimeError): + await actions.approve_playbook(agent_id=_APPROVER, playbook_id=_PID) + + svc.index_approved.assert_not_awaited() + assert order == ["approve"] + + +@pytest.mark.asyncio +async def test_gateway_reject_commits_before_unindexing( + monkeypatch: pytest.MonkeyPatch, +) -> None: + order: list[str] = [] + actions, _task = _gateway_actions(order) + svc = _gateway_svc_mock(order) + monkeypatch.setattr( + "roboco.services.playbook.get_playbook_service", + lambda _session: svc, + ) + + env = await actions.reject_playbook( + agent_id=_APPROVER, playbook_id=_PID, reason="nope" + ) + + assert order == ["reject", "commit", "unindex"] + assert env.status == "playbook_archived" diff --git a/tests/unit/services/test_playbook_unindex.py b/tests/unit/services/test_playbook_unindex.py index 697697fd..1993dc3d 100644 --- a/tests/unit/services/test_playbook_unindex.py +++ b/tests/unit/services/test_playbook_unindex.py @@ -1,11 +1,15 @@ -"""F011 — PlaybookService.reject must de-index the playbook. +"""PlaybookService.reject must flush the status change WITHOUT de-indexing +inline, and ``unindex_playbook`` is the separate post-commit de-index step. -A rejected/archived playbook that was previously approved stays in the -PLAYBOOKS RAG index (no de-index on reject/archive), so it keeps surfacing -in agent briefings as a stale, no-longer-canonical procedure. The fix -mirrors the index-on-approve path: ``reject`` calls an -``_unindex_playbook`` helper (gated on ``org_memory_enabled``, best-effort) -that removes the playbook's chunks + tracking row. +A rejected/archived playbook that was previously approved must stop surfacing in +agent briefings, so a reject drops its chunks + tracking row. Originally (F011) +``reject`` called an ``_unindex_playbook`` helper inline. F057 split that out: +the RAG index write runs through its own auto-committing pool connection, so +de-indexing inline (before the caller commits the status) would drop a playbook +from the corpus even if the status transaction rolled back — a divergence. So +``reject`` now flushes the status ONLY; ``unindex_playbook`` is a separate +public step the caller runs AFTER committing. Both helpers stay gated on +``org_memory_enabled`` (inert when the loop is off) and best-effort. """ from __future__ import annotations @@ -32,26 +36,28 @@ def _mock_playbook(playbook_id, *, status=PlaybookStatus.APPROVED.value): return pb -@pytest.mark.asyncio -async def test_reject_deindexes_approved_playbook( - monkeypatch: pytest.MonkeyPatch, -) -> None: - """Rejecting an approved (indexed) playbook removes it from the index.""" - monkeypatch.setattr(settings, "org_memory_enabled", True) - playbook_id = uuid4() - pb = _mock_playbook(playbook_id, status=PlaybookStatus.APPROVED.value) - +def _session_with(pb) -> MagicMock: session = MagicMock() - # _get_or_raise returns the playbook; flush is a no-op. result = MagicMock() result.scalar_one_or_none.return_value = pb session.execute = AsyncMock(return_value=result) session.flush = AsyncMock() + return session + + +@pytest.mark.asyncio +async def test_reject_archives_but_does_not_deindex_inline( + monkeypatch: pytest.MonkeyPatch, +) -> None: + """``reject`` flushes the ARCHIVED status but must NOT touch the RAG index — + the de-index is a separate post-commit step (F057 ordering).""" + monkeypatch.setattr(settings, "org_memory_enabled", True) + playbook_id = uuid4() + pb = _mock_playbook(playbook_id, status=PlaybookStatus.APPROVED.value) + svc = PlaybookService(_session_with(pb)) optimal = MagicMock() optimal.unindex_playbook = AsyncMock(return_value=None) - - svc = PlaybookService(session) with ( patch( "roboco.services.optimal.get_optimal_service", @@ -61,68 +67,52 @@ async def test_reject_deindexes_approved_playbook( out = await svc.reject(playbook_id, approver_id=uuid4(), reason="stale") assert out.status == PlaybookStatus.ARCHIVED.value - optimal.unindex_playbook.assert_awaited_once_with(str(playbook_id)) + optimal.unindex_playbook.assert_not_awaited() @pytest.mark.asyncio -async def test_reject_of_unindexed_draft_does_not_error( +async def test_unindex_playbook_deindexes_approved_playbook( monkeypatch: pytest.MonkeyPatch, ) -> None: - """A draft (never indexed) reject still de-indexes (idempotent no-op) and - never errors the curation.""" + """``unindex_playbook`` (the post-commit step) removes an approved playbook + from the index.""" monkeypatch.setattr(settings, "org_memory_enabled", True) playbook_id = uuid4() - pb = _mock_playbook(playbook_id, status=PlaybookStatus.DRAFT.value) - - session = MagicMock() - result = MagicMock() - result.scalar_one_or_none.return_value = pb - session.execute = AsyncMock(return_value=result) - session.flush = AsyncMock() + pb = _mock_playbook(playbook_id, status=PlaybookStatus.APPROVED.value) + svc = PlaybookService(_session_with(pb)) optimal = MagicMock() optimal.unindex_playbook = AsyncMock(return_value=None) - - svc = PlaybookService(session) with ( patch( "roboco.services.optimal.get_optimal_service", AsyncMock(return_value=optimal), ), ): - out = await svc.reject(playbook_id, approver_id=uuid4(), reason="nope") + await svc.unindex_playbook(pb) - assert out.status == PlaybookStatus.ARCHIVED.value - # De-index is still called (idempotent — the store no-ops on an absent - # source), so a draft reject mirrors the approved reject path uniformly. optimal.unindex_playbook.assert_awaited_once_with(str(playbook_id)) @pytest.mark.asyncio -async def test_reject_skips_deindex_when_org_memory_disabled( +async def test_unindex_playbook_skips_when_org_memory_disabled( monkeypatch: pytest.MonkeyPatch, ) -> None: - """org_memory_enabled=False ⇒ the index is inert; reject must not touch it.""" + """org_memory_enabled=False ⇒ the index is inert; ``unindex_playbook`` must + not touch it.""" monkeypatch.setattr(settings, "org_memory_enabled", False) playbook_id = uuid4() pb = _mock_playbook(playbook_id) - - session = MagicMock() - result = MagicMock() - result.scalar_one_or_none.return_value = pb - session.execute = AsyncMock(return_value=result) - session.flush = AsyncMock() + svc = PlaybookService(_session_with(pb)) optimal = MagicMock() optimal.unindex_playbook = AsyncMock(return_value=None) - - svc = PlaybookService(session) with ( patch( "roboco.services.optimal.get_optimal_service", AsyncMock(return_value=optimal), ), ): - await svc.reject(playbook_id, approver_id=uuid4(), reason="nope") + await svc.unindex_playbook(pb) optimal.unindex_playbook.assert_not_awaited()