diff --git a/api/services/skill_management_service.py b/api/services/skill_management_service.py index 8db94c4ec85..71ae5eb27b2 100644 --- a/api/services/skill_management_service.py +++ b/api/services/skill_management_service.py @@ -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 ( diff --git a/api/tests/unit_tests/services/test_skill_management_service.py b/api/tests/unit_tests/services/test_skill_management_service.py index 9521646ff7f..bc4d5dafa82 100644 --- a/api/tests/unit_tests/services/test_skill_management_service.py +++ b/api/tests/unit_tests/services/test_skill_management_service.py @@ -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: diff --git a/web/features/agent-v2/agent-detail/configure/components/orchestrate/add-actions.tsx b/web/features/agent-v2/agent-detail/configure/components/orchestrate/add-actions.tsx index 7617916895f..847600d750e 100644 --- a/web/features/agent-v2/agent-detail/configure/components/orchestrate/add-actions.tsx +++ b/web/features/agent-v2/agent-detail/configure/components/orchestrate/add-actions.tsx @@ -8,15 +8,15 @@ import type { } from './add-actions-context' import { useCallback, useMemo, useState } from 'react' import { AgentOrchestrateAddActionsContext } from './add-actions-context' -import { useAgentOrchestrateReadOnly } from './read-only-context' +import { useAgentOrchestrateViewingVersion } from './read-only-context' export function AgentOrchestrateAddActionsProvider({ children }: { children: ReactNode }) { - const readOnly = useAgentOrchestrateReadOnly() + const isViewingVersion = useAgentOrchestrateViewingVersion() const [actions, setActions] = useState({}) const registerAction = useCallback( (key: AgentOrchestrateAddActionKey, action: AgentOrchestrateAddAction) => { - if (readOnly) return () => undefined + if (isViewingVersion) return () => undefined setActions((currentActions) => { if (currentActions[key] === action) return currentActions @@ -37,15 +37,15 @@ export function AgentOrchestrateAddActionsProvider({ children }: { children: Rea }) } }, - [readOnly], + [isViewingVersion], ) const value = useMemo( () => ({ - actions: readOnly ? {} : actions, + actions: isViewingVersion ? {} : actions, registerAction, }), - [actions, readOnly, registerAction], + [actions, isViewingVersion, registerAction], ) return ( diff --git a/web/features/agent-v2/agent-detail/configure/components/orchestrate/common/add-button.tsx b/web/features/agent-v2/agent-detail/configure/components/orchestrate/common/add-button.tsx index 98530767e4c..d70c33b9389 100644 --- a/web/features/agent-v2/agent-detail/configure/components/orchestrate/common/add-button.tsx +++ b/web/features/agent-v2/agent-detail/configure/components/orchestrate/common/add-button.tsx @@ -4,7 +4,7 @@ import type { ButtonProps } from '@langgenius/dify-ui/button' import { Button } from '@langgenius/dify-ui/button' import { cn } from '@langgenius/dify-ui/cn' import { useTranslation } from 'react-i18next' -import { useAgentOrchestrateReadOnly } from '../read-only-context' +import { useAgentOrchestrateViewingVersion } from '../read-only-context' type ConfigureSectionAddButtonProps = Omit< ButtonProps, @@ -19,9 +19,9 @@ export function ConfigureSectionAddButton({ ...props }: ConfigureSectionAddButtonProps) { const { t } = useTranslation('common') - const readOnly = useAgentOrchestrateReadOnly() + const isViewingVersion = useAgentOrchestrateViewingVersion() - if (readOnly) return null + if (isViewingVersion) return null return (