diff --git a/navi/api/websocket.py b/navi/api/websocket.py index 758104e..3862111 100644 --- a/navi/api/websocket.py +++ b/navi/api/websocket.py @@ -563,6 +563,7 @@ proposal_id, accepted=action == "accept", user_id=actor_id, + is_admin=is_admin, ) await websocket.send_json({ "type": "skill_proposal_decided", diff --git a/navi/skills/store.py b/navi/skills/store.py index ef3bcb3..7105cd7 100644 --- a/navi/skills/store.py +++ b/navi/skills/store.py @@ -574,15 +574,19 @@ *, accepted: bool, user_id: str = "", + is_admin: bool = False, ) -> tuple[SkillProposal | None, SkillEntry | None, str]: """Accept or reject a proposal. Accepting writes the catalog row (or a new version of the protocol the proposal replaces) with ``user_id`` as the author — the user's click is the authorization, so the version history names the person who agreed to - the procedure. Returns ``(proposal, entry, error)``: an unknown id gives - ``(None, None, message)``; a document the store refuses leaves the - proposal pending and returns the reason. + the procedure. A replacement is authorized again at the click: the + protocol may have changed hands (a personal one promoted to shared) while + the proposal sat pending. Returns ``(proposal, entry, error)``: an unknown + id gives ``(None, None, message)``; a document — or a protocol the actor + may no longer write — the store refuses leaves the proposal pending and + returns the reason. """ await self._ensure_ready() proposal = await self.get_proposal(proposal_id) @@ -599,6 +603,23 @@ try: if proposal.replaces_id: + # Ownership was proven when the proposal was written; it has to + # hold at the click too, because the entry can be promoted to + # shared (losing its owner) in the meantime. A shared protocol + # is then the hive's, and only an admin may replace it. + current = await self.get(proposal.replaces_id) + if current is None or current.kind != "protocol": + raise SkillError("the protocol this proposal would replace is gone") + if current.visibility == "shared": + if not is_admin: + raise SkillError( + "the protocol is shared now; only an admin can replace it" + ) + elif current.owner_id != user_id: + raise SkillError( + "the protocol changed hands while this proposal was pending; " + "only its owner can replace it" + ) entry = await self.update( proposal.replaces_id, content=proposal.content, diff --git a/tests/unit/skills/test_skill_store.py b/tests/unit/skills/test_skill_store.py index e2bcde0..d0a8f7e 100644 --- a/tests/unit/skills/test_skill_store.py +++ b/tests/unit/skills/test_skill_store.py @@ -321,6 +321,7 @@ async def test_accepting_a_replacement_bumps_that_protocol(self): store, conn = _store() conn.enqueue(self._proposal_row(replaces_id="sk1")) + conn.enqueue(_row(kind="protocol", version=1)) # ownership re-check conn.enqueue(_row(kind="protocol", version=1)) # current conn.enqueue("OK") # history row conn.enqueue("OK") # update skills @@ -333,6 +334,42 @@ assert entry.version == 2 assert any("UPDATE skills SET" in c[1] for c in conn.calls) + async def test_a_replacement_promoted_to_shared_needs_an_admin(self): + # The owner proposed a change while the protocol was personal; an admin + # then promoted it to the hive. The owner's click must no longer write. + store, conn = _store() + conn.enqueue(self._proposal_row(replaces_id="sk1")) + conn.enqueue(_row(kind="protocol", visibility="shared", owner_id="")) + conn.enqueue("OK") # store the error text + conn.enqueue(self._proposal_row( + replaces_id="sk1", error="the protocol is shared now; only an admin", + )) + + _, entry, error = await store.decide_proposal("pr1", accepted=True, user_id="u1") + + assert entry is None + assert "only an admin can replace it" in error + assert not any("UPDATE skills SET" in c[1] for c in conn.calls) + + async def test_an_admin_still_accepts_a_replacement_of_a_shared_protocol(self): + store, conn = _store() + conn.enqueue(self._proposal_row(replaces_id="sk1")) + conn.enqueue(_row(kind="protocol", visibility="shared", owner_id="")) + conn.enqueue(_row(kind="protocol", visibility="shared", owner_id="", version=1)) + conn.enqueue("OK") # history row + conn.enqueue("OK") # update skills + conn.enqueue(_row(kind="protocol", visibility="shared", owner_id="", version=2)) + conn.enqueue("OK") # mark accepted + conn.enqueue(self._proposal_row(status="accepted", replaces_id="sk1", skill_id="sk1")) + + _, entry, error = await store.decide_proposal( + "pr1", accepted=True, user_id="admin1", is_admin=True, + ) + + assert error == "" + assert entry.version == 2 + assert any("UPDATE skills SET" in c[1] for c in conn.calls) + async def test_rejecting_leaves_the_catalog_untouched(self): store, conn = _store() conn.enqueue(self._proposal_row())