diff --git a/navi/skills/store.py b/navi/skills/store.py index 28280d9..ef3bcb3 100644 --- a/navi/skills/store.py +++ b/navi/skills/store.py @@ -613,6 +613,10 @@ content=proposal.content, edited_by=user_id, reason="accepted proposal", + # The proposal was parsed as a protocol when it was written; + # its front matter may leave `kind` out, so name it here too + # instead of failing on a document we already accepted. + kind="protocol", ) except SkillError as e: async with self._pool.acquire() as conn: diff --git a/navi/tools/skills.py b/navi/tools/skills.py index 247e536..cb3de7d 100644 --- a/navi/tools/skills.py +++ b/navi/tools/skills.py @@ -214,7 +214,15 @@ try: doc = parse_document(content) except SkillError as e: - return ToolResult(success=False, output="", error=str(e)) + # The op already says what this document is, so a skill whose front + # matter leaves `kind` out is still a skill. Only that refusal gets a + # second look; anything else is reported as written. + if "'kind' is required" not in str(e): + return ToolResult(success=False, output="", error=str(e)) + try: + doc = parse_document(content, kind="skill") + except SkillError as inner: + return ToolResult(success=False, output="", error=str(inner)) if doc.kind == "protocol": return ToolResult( success=False, output="", @@ -226,7 +234,7 @@ try: entry = await self._store.create( owner_id=user_id, content=content, edited_by=user_id, - reason="written by the agent", + reason="written by the agent", kind="skill", ) except SkillError as e: return ToolResult(success=False, output="", error=str(e)) diff --git a/tests/unit/skills/test_skill_store.py b/tests/unit/skills/test_skill_store.py index 9d935d7..e2bcde0 100644 --- a/tests/unit/skills/test_skill_store.py +++ b/tests/unit/skills/test_skill_store.py @@ -30,6 +30,19 @@ --- """ +# A model often writes the front matter without the kind, and the proposal is +# parsed with kind='protocol' when it is stored; accepting it must not then +# fail on the very document the store already read. +PROTOCOL_NO_KIND = """--- +name: Выключение +steps: [Раз, Два] +post_checks: [Готово] +on_deviation: Стоп. +on_blocked: Стоп и доложить. +on_already_done: Проверить. +--- +""" + def _store(conn: FakeConnection | None = None) -> tuple[SkillStore, FakeConnection]: conn = conn or FakeConnection() @@ -286,6 +299,25 @@ assert update[2][1] == "accepted" assert update[2][3] == "u1" + async def test_accepting_a_proposal_whose_document_omits_the_kind(self): + store, conn = _store() + conn.enqueue(self._proposal_row(content=PROTOCOL_NO_KIND)) + conn.enqueue("OK") # INSERT INTO skills + conn.enqueue("OK") # INSERT INTO skill_versions + conn.enqueue(_row(kind="protocol", content=PROTOCOL_NO_KIND, name="Выключение")) + conn.enqueue("OK") # mark accepted + conn.enqueue(self._proposal_row( + content=PROTOCOL_NO_KIND, status="accepted", skill_id="sk1", + )) + + proposal, entry, error = await store.decide_proposal( + "pr1", accepted=True, user_id="u1", + ) + assert error == "" + assert entry is not None and entry.kind == "protocol" + assert proposal.status == "accepted" + assert not any("UPDATE skill_proposals SET error" in c[1] for c in conn.calls) + async def test_accepting_a_replacement_bumps_that_protocol(self): store, conn = _store() conn.enqueue(self._proposal_row(replaces_id="sk1")) diff --git a/tests/unit/tools/test_skills.py b/tests/unit/tools/test_skills.py index d5e8f29..56f8e8c 100644 --- a/tests/unit/tools/test_skills.py +++ b/tests/unit/tools/test_skills.py @@ -36,8 +36,8 @@ """ -def _entry(content: str, **kw) -> SkillEntry: - doc = parse_document(content) +def _entry(content: str, *, kind: str | None = None, **kw) -> SkillEntry: + doc = parse_document(content, kind=kind) base = dict( id=kw.pop("id", doc.kind + "-1"), kind=doc.kind, @@ -104,7 +104,7 @@ async def create(self, *, owner_id, content, visibility="personal", edited_by="", reason=None, kind=None): self.calls.append(("create", owner_id, content)) - entry = _entry(content, owner_id=owner_id) + entry = _entry(content, kind=kind, owner_id=owner_id) self.entries.append(entry) return entry @@ -211,6 +211,15 @@ assert "op='propose'" in result.error assert store.calls == [] + async def test_create_accepts_a_skill_whose_front_matter_omits_kind(self, ctx): + store = FakeStore([]) + content = SKILL_DOC.replace("kind: skill\n", "") + result = await SkillTool(store).execute( + {"op": "create", "content": content}, ctx, + ) + assert result.success + assert result.metadata["kind"] == "skill" + assert store.calls[-1][0] == "create" async def test_create_reports_a_malformed_document(self, ctx): store = FakeStore([]) result = await SkillTool(store).execute(