fix: fix delete skill ref
This commit is contained in:
@@ -1247,6 +1247,13 @@ class SkillManagementService:
|
||||
"skill is referenced and requires name confirmation",
|
||||
status_code=409,
|
||||
)
|
||||
self._remove_skill_reference_consumers(
|
||||
session,
|
||||
tenant_id=tenant_id,
|
||||
skill=skill,
|
||||
user_id=skill.updated_by,
|
||||
updated_at=naive_utc_now(),
|
||||
)
|
||||
session.query(AgentSkillBinding).filter(
|
||||
AgentSkillBinding.tenant_id == tenant_id,
|
||||
AgentSkillBinding.skill_id == skill.id,
|
||||
@@ -1998,6 +2005,121 @@ class SkillManagementService:
|
||||
binding.updated_by = user_id
|
||||
binding.updated_at = updated_at
|
||||
|
||||
def _remove_skill_reference_consumers(
|
||||
self,
|
||||
session,
|
||||
*,
|
||||
tenant_id: str,
|
||||
skill: Skill,
|
||||
user_id: str,
|
||||
updated_at,
|
||||
) -> None:
|
||||
agents = list(
|
||||
session.scalars(
|
||||
select(Agent)
|
||||
.join(AgentSkillBinding, AgentSkillBinding.agent_id == Agent.id)
|
||||
.where(
|
||||
AgentSkillBinding.tenant_id == tenant_id,
|
||||
AgentSkillBinding.skill_id == skill.id,
|
||||
Agent.tenant_id == tenant_id,
|
||||
)
|
||||
)
|
||||
)
|
||||
if not agents:
|
||||
return
|
||||
|
||||
workflow_bindings_by_agent_id = self._workflow_inline_bindings_by_agent_id(
|
||||
session,
|
||||
tenant_id=tenant_id,
|
||||
agent_ids=[agent.id for agent in agents],
|
||||
)
|
||||
for agent in agents:
|
||||
self._remove_agent_config_skill_ref(
|
||||
session,
|
||||
agent=agent,
|
||||
skill_name=skill.name,
|
||||
user_id=user_id,
|
||||
updated_at=updated_at,
|
||||
workflow_bindings=workflow_bindings_by_agent_id.get(agent.id, []),
|
||||
)
|
||||
agent.updated_by = user_id
|
||||
agent.updated_at = updated_at
|
||||
|
||||
def _remove_agent_config_skill_ref(
|
||||
self,
|
||||
session,
|
||||
*,
|
||||
agent: Agent,
|
||||
skill_name: str,
|
||||
user_id: str,
|
||||
updated_at,
|
||||
workflow_bindings: list[WorkflowAgentNodeBinding],
|
||||
) -> None:
|
||||
new_snapshot_id: str | None = None
|
||||
previous_active_snapshot_id = agent.active_config_snapshot_id
|
||||
if previous_active_snapshot_id:
|
||||
active_snapshot = session.scalar(
|
||||
select(AgentConfigSnapshot).where(
|
||||
AgentConfigSnapshot.tenant_id == agent.tenant_id,
|
||||
AgentConfigSnapshot.agent_id == agent.id,
|
||||
AgentConfigSnapshot.id == previous_active_snapshot_id,
|
||||
)
|
||||
)
|
||||
if active_snapshot is not None:
|
||||
agent_soul = AgentSoulConfig.model_validate(active_snapshot.config_snapshot_dict)
|
||||
agent_soul.config_skills = self._remove_config_skill_ref(agent_soul.config_skills, skill_name)
|
||||
new_snapshot = AgentConfigSnapshot(
|
||||
tenant_id=agent.tenant_id,
|
||||
agent_id=agent.id,
|
||||
version=self._next_agent_config_version(session, tenant_id=agent.tenant_id, agent_id=agent.id),
|
||||
config_snapshot=agent_soul,
|
||||
version_note=f"Removed workspace skill {skill_name}",
|
||||
created_by=user_id,
|
||||
)
|
||||
session.add(new_snapshot)
|
||||
session.flush()
|
||||
session.add(
|
||||
AgentConfigRevision(
|
||||
tenant_id=agent.tenant_id,
|
||||
agent_id=agent.id,
|
||||
previous_snapshot_id=active_snapshot.id,
|
||||
current_snapshot_id=new_snapshot.id,
|
||||
revision=self._next_agent_config_revision(
|
||||
session,
|
||||
tenant_id=agent.tenant_id,
|
||||
agent_id=agent.id,
|
||||
),
|
||||
operation=AgentConfigRevisionOperation.SAVE_CURRENT_VERSION,
|
||||
version_note=f"Removed workspace skill {skill_name}",
|
||||
created_by=user_id,
|
||||
)
|
||||
)
|
||||
agent.active_config_snapshot_id = new_snapshot.id
|
||||
new_snapshot_id = new_snapshot.id
|
||||
|
||||
drafts = list(
|
||||
session.scalars(
|
||||
select(AgentConfigDraft).where(
|
||||
AgentConfigDraft.tenant_id == agent.tenant_id,
|
||||
AgentConfigDraft.agent_id == agent.id,
|
||||
)
|
||||
)
|
||||
)
|
||||
for draft in drafts:
|
||||
draft_soul = AgentSoulConfig.model_validate(draft.config_snapshot_dict)
|
||||
draft_soul.config_skills = self._remove_config_skill_ref(draft_soul.config_skills, skill_name)
|
||||
draft.config_snapshot = draft_soul
|
||||
if new_snapshot_id and draft.base_snapshot_id == previous_active_snapshot_id:
|
||||
draft.base_snapshot_id = new_snapshot_id
|
||||
draft.updated_by = user_id
|
||||
draft.updated_at = updated_at
|
||||
|
||||
for binding in workflow_bindings:
|
||||
if new_snapshot_id:
|
||||
binding.current_snapshot_id = new_snapshot_id
|
||||
binding.updated_by = user_id
|
||||
binding.updated_at = updated_at
|
||||
|
||||
@staticmethod
|
||||
def _upsert_config_skill_ref(
|
||||
current: list[AgentConfigSkillRefConfig],
|
||||
@@ -2010,6 +2132,13 @@ class SkillManagementService:
|
||||
by_name[skill_ref.name] = skill_ref
|
||||
return [by_name[name] for name in order if name in by_name]
|
||||
|
||||
@staticmethod
|
||||
def _remove_config_skill_ref(
|
||||
current: list[AgentConfigSkillRefConfig],
|
||||
skill_name: str,
|
||||
) -> list[AgentConfigSkillRefConfig]:
|
||||
return [item for item in current if item.name != skill_name]
|
||||
|
||||
@staticmethod
|
||||
def _next_agent_config_version(session, *, tenant_id: str, agent_id: str) -> int:
|
||||
return (
|
||||
|
||||
@@ -10,7 +10,7 @@ from unittest.mock import patch
|
||||
from uuid import uuid4
|
||||
|
||||
import pytest
|
||||
from sqlalchemy import delete, select
|
||||
from sqlalchemy import delete, func, select
|
||||
|
||||
from core.db.session_factory import session_factory
|
||||
from models.account import Account
|
||||
@@ -1255,6 +1255,28 @@ def test_duplicate_skill_copies_latest_published_content_without_history() -> No
|
||||
assert service.list_versions(tenant_id=TENANT, skill_id=duplicated["id"]) == {"data": []}
|
||||
|
||||
|
||||
def test_duplicate_skill_does_not_copy_agent_references() -> None:
|
||||
service = SkillManagementService(tool_file_manager=_FakeToolFileManager())
|
||||
created = service.create_skill(
|
||||
tenant_id=TENANT,
|
||||
user_id=USER,
|
||||
payload=SkillCreatePayload(name="finance-sop"),
|
||||
)
|
||||
service.replace_agent_bindings(tenant_id=TENANT, user_id=USER, agent_id=AGENT, skill_ids=[created["id"]])
|
||||
|
||||
duplicated = service.duplicate_skill(tenant_id=TENANT, user_id=USER, skill_id=created["id"])
|
||||
|
||||
assert duplicated["reference_count"] == 0
|
||||
assert service.list_skill_references(tenant_id=TENANT, skill_id=duplicated["id"]) == {"data": []}
|
||||
assert service.list_agent_bindings(tenant_id=TENANT, agent_id=AGENT)["skill_ids"] == [created["id"]]
|
||||
listed = service.list_skills(tenant_id=TENANT, keyword=None, tags=[], page=1, limit=10)
|
||||
ref_counts_by_name = {skill["name"]: skill["reference_count"] for skill in listed["data"]}
|
||||
assert ref_counts_by_name == {
|
||||
"finance-sop": 1,
|
||||
"finance-sop-copy": 0,
|
||||
}
|
||||
|
||||
|
||||
def test_duplicate_unpublished_skill_copies_current_draft() -> None:
|
||||
service = SkillManagementService(tool_file_manager=_FakeToolFileManager())
|
||||
created = service.create_skill(tenant_id=TENANT, user_id=USER, payload=SkillCreatePayload(name="finance-sop"))
|
||||
@@ -1286,6 +1308,94 @@ def test_delete_skill_requires_confirmation_when_referenced() -> None:
|
||||
assert service.list_skills(tenant_id=TENANT)["data"] == []
|
||||
|
||||
|
||||
def test_delete_skill_removes_synced_agent_config_skill_refs() -> None:
|
||||
service = SkillManagementService(tool_file_manager=_FakeToolFileManager())
|
||||
created = service.create_skill(tenant_id=TENANT, user_id=USER, payload=SkillCreatePayload(name="finance-sop"))
|
||||
with session_factory.create_session() as session:
|
||||
agent_snapshot = AgentConfigSnapshot(
|
||||
tenant_id=TENANT,
|
||||
agent_id=AGENT,
|
||||
version=1,
|
||||
config_snapshot=AgentSoulConfig(
|
||||
config_skills=[
|
||||
AgentConfigSkillRefConfig(
|
||||
name="finance-sop",
|
||||
description="Finance SOP",
|
||||
file_id="workspace-skill-file",
|
||||
size=1,
|
||||
hash="workspace-skill-hash",
|
||||
),
|
||||
AgentConfigSkillRefConfig(
|
||||
name="inline-helper",
|
||||
description="Inline helper",
|
||||
file_id="inline-skill-file",
|
||||
size=1,
|
||||
hash="inline-skill-hash",
|
||||
),
|
||||
]
|
||||
),
|
||||
created_by=USER,
|
||||
)
|
||||
session.add(agent_snapshot)
|
||||
session.flush()
|
||||
agent = session.get(Agent, AGENT)
|
||||
assert agent is not None
|
||||
agent.active_config_snapshot_id = agent_snapshot.id
|
||||
session.add(
|
||||
AgentConfigDraft(
|
||||
tenant_id=TENANT,
|
||||
agent_id=AGENT,
|
||||
draft_type=AgentConfigDraftType.DRAFT,
|
||||
account_id=None,
|
||||
draft_owner_key="",
|
||||
base_snapshot_id=agent_snapshot.id,
|
||||
config_snapshot=AgentSoulConfig(
|
||||
config_skills=[
|
||||
AgentConfigSkillRefConfig(
|
||||
name="finance-sop",
|
||||
description="Finance SOP",
|
||||
file_id="workspace-skill-file",
|
||||
size=1,
|
||||
hash="workspace-skill-hash",
|
||||
)
|
||||
]
|
||||
),
|
||||
created_by=USER,
|
||||
updated_by=USER,
|
||||
)
|
||||
)
|
||||
session.commit()
|
||||
|
||||
service.replace_agent_bindings(tenant_id=TENANT, user_id=USER, agent_id=AGENT, skill_ids=[created["id"]])
|
||||
|
||||
deleted = service.delete_skill(tenant_id=TENANT, skill_id=created["id"], confirmation_name="finance-sop")
|
||||
|
||||
with session_factory.create_session() as session:
|
||||
agent = session.get(Agent, AGENT)
|
||||
assert agent is not None
|
||||
active_snapshot = session.get(AgentConfigSnapshot, agent.active_config_snapshot_id)
|
||||
draft = session.scalar(
|
||||
select(AgentConfigDraft).where(
|
||||
AgentConfigDraft.agent_id == AGENT,
|
||||
AgentConfigDraft.draft_type == AgentConfigDraftType.DRAFT,
|
||||
)
|
||||
)
|
||||
binding_count = session.scalar(select(func.count()).select_from(AgentSkillBinding))
|
||||
|
||||
assert deleted == {"id": created["id"], "deleted": True}
|
||||
assert active_snapshot is not None
|
||||
assert draft is not None
|
||||
assert binding_count == 0
|
||||
assert active_snapshot.version == 2
|
||||
active_skill_names = [
|
||||
item.name for item in AgentSoulConfig.model_validate(active_snapshot.config_snapshot_dict).config_skills
|
||||
]
|
||||
draft_skill_names = [item.name for item in AgentSoulConfig.model_validate(draft.config_snapshot_dict).config_skills]
|
||||
assert active_skill_names == ["inline-helper"]
|
||||
assert draft_skill_names == []
|
||||
assert draft.base_snapshot_id == active_snapshot.id
|
||||
|
||||
|
||||
def test_import_skill_package_creates_draft_and_rejects_name_conflicts() -> None:
|
||||
package = io.BytesIO()
|
||||
with zipfile.ZipFile(package, "w") as archive:
|
||||
|
||||
Reference in New Issue
Block a user