Without this listener, deleting a skill from the skill management page + * leaves orphan binding rows behind: + *
The event is dispatched synchronously from {@code SkillService} after
+ * the {@code mate_skill} row deletion, so the cleanup is part of the same
+ * request and observable in the very next list call.
+ */
+@Slf4j
+@Component
+@RequiredArgsConstructor
+public class AgentBindingSkillRemovalListener {
+
+ private final AgentSkillBindingMapper skillBindingMapper;
+
+ @EventListener
+ public void onSkillRemoved(SkillRemovedEvent event) {
+ if (event == null || event.skillId() == null) {
+ return;
+ }
+ int dropped = skillBindingMapper.delete(
+ new LambdaQueryWrapper Downstream listeners use this to scrub records that reference the
+ * deleted skill — most importantly the agent-skill binding rows in
+ * {@code mate_agent_skill}, which would otherwise leave orphan bindings the
+ * UI can't unset (the binding count stays > 0 and the picker can no longer
+ * render the row to uncheck it).
+ *
+ * @param skillId DB id of the removed skill row
+ * @param skillName slug identifier the row carried, useful for log lines
+ */
+public record SkillRemovedEvent(Long skillId, String skillName) {
+}
diff --git a/mateclaw-server/src/main/java/vip/mate/skill/service/SkillService.java b/mateclaw-server/src/main/java/vip/mate/skill/service/SkillService.java
index f20b8612..82310f5e 100644
--- a/mateclaw-server/src/main/java/vip/mate/skill/service/SkillService.java
+++ b/mateclaw-server/src/main/java/vip/mate/skill/service/SkillService.java
@@ -5,8 +5,10 @@ import com.baomidou.mybatisplus.core.metadata.IPage;
import com.baomidou.mybatisplus.extension.plugins.pagination.Page;
import lombok.RequiredArgsConstructor;
import lombok.extern.slf4j.Slf4j;
+import org.springframework.context.ApplicationEventPublisher;
import org.springframework.stereotype.Service;
import vip.mate.exception.MateClawException;
+import vip.mate.skill.event.SkillRemovedEvent;
import vip.mate.skill.model.SkillEntity;
import vip.mate.skill.repository.SkillFileMapper;
import vip.mate.skill.repository.SkillMapper;
@@ -47,6 +49,13 @@ public class SkillService {
private final SkillWorkspaceManager workspaceManager;
private final SkillWorkspaceProperties workspaceProperties;
private final SkillSecretService skillSecretService;
+ /**
+ * Fires {@link SkillRemovedEvent} on both uninstall and hard-delete so
+ * the agent-binding listener (and any future subscriber) can scrub
+ * dependent rows — without this, {@code mate_agent_skill} keeps orphan
+ * rows that the UI can no longer clear from the picker.
+ */
+ private final ApplicationEventPublisher eventPublisher;
private vip.mate.skill.runtime.SkillRuntimeService runtimeService;
/**
@@ -414,6 +423,10 @@ public class SkillService {
skillMapper.deleteById(id); // logical delete (deleted=1)
log.info("Uninstalled skill (logical delete + archive): {}", skill.getName());
+ // Notify listeners (e.g. agent-binding cleanup) so dependent rows
+ // referencing this skill_id don't outlive the row itself.
+ eventPublisher.publishEvent(new SkillRemovedEvent(id, skill.getName()));
+
if ("archive".equals(workspaceProperties.getDeletePolicy())) {
workspaceManager.archiveWorkspace(skill.getName());
}
@@ -448,6 +461,10 @@ public class SkillService {
}
log.info("Hard-deleted skill (physical delete + purge): {}", skill.getName());
+ // Same notification as the uninstall path — agent-binding cleanup
+ // applies regardless of which delete flavor the admin chose.
+ eventPublisher.publishEvent(new SkillRemovedEvent(id, skill.getName()));
+
// RFC-091 settings bridge — purge any per-skill secrets so a
// future skill reusing this id doesn't inherit stale credentials.
try {
diff --git a/mateclaw-server/src/test/java/vip/mate/agent/binding/AgentBindingSkillRemovalListenerTest.java b/mateclaw-server/src/test/java/vip/mate/agent/binding/AgentBindingSkillRemovalListenerTest.java
new file mode 100644
index 00000000..36a7cbf2
--- /dev/null
+++ b/mateclaw-server/src/test/java/vip/mate/agent/binding/AgentBindingSkillRemovalListenerTest.java
@@ -0,0 +1,49 @@
+package vip.mate.agent.binding;
+
+import com.baomidou.mybatisplus.core.conditions.query.LambdaQueryWrapper;
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+import vip.mate.agent.binding.model.AgentSkillBinding;
+import vip.mate.agent.binding.repository.AgentSkillBindingMapper;
+import vip.mate.agent.binding.service.AgentBindingSkillRemovalListener;
+import vip.mate.skill.event.SkillRemovedEvent;
+
+import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.never;
+import static org.mockito.Mockito.times;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+/**
+ * Issue #127 regression — deleting a skill from the skill management page
+ * left orphan rows in {@code mate_agent_skill}, so the agent edit modal kept
+ * the old binding count and the user couldn't clear it. This listener drops
+ * those rows in response to {@link SkillRemovedEvent}.
+ */
+class AgentBindingSkillRemovalListenerTest {
+
+ @Test
+ @DisplayName("event triggers a delete on mate_agent_skill scoped to the removed skill id")
+ void removalDropsBindingRows() {
+ AgentSkillBindingMapper mapper = mock(AgentSkillBindingMapper.class);
+ when(mapper.delete(any(LambdaQueryWrapper.class))).thenReturn(2);
+
+ AgentBindingSkillRemovalListener listener = new AgentBindingSkillRemovalListener(mapper);
+ listener.onSkillRemoved(new SkillRemovedEvent(77L, "pdf"));
+
+ verify(mapper, times(1)).delete(any(LambdaQueryWrapper.class));
+ }
+
+ @Test
+ @DisplayName("null event or null skillId is a no-op — defensive guard")
+ void nullEventDoesNothing() {
+ AgentSkillBindingMapper mapper = mock(AgentSkillBindingMapper.class);
+ AgentBindingSkillRemovalListener listener = new AgentBindingSkillRemovalListener(mapper);
+
+ listener.onSkillRemoved(null);
+ listener.onSkillRemoved(new SkillRemovedEvent(null, "dangling"));
+
+ verify(mapper, never()).delete(any());
+ }
+}
diff --git a/mateclaw-server/src/test/java/vip/mate/skill/service/SkillServiceRemovalEventTest.java b/mateclaw-server/src/test/java/vip/mate/skill/service/SkillServiceRemovalEventTest.java
new file mode 100644
index 00000000..c1d9c130
--- /dev/null
+++ b/mateclaw-server/src/test/java/vip/mate/skill/service/SkillServiceRemovalEventTest.java
@@ -0,0 +1,94 @@
+package vip.mate.skill.service;
+
+import org.junit.jupiter.api.DisplayName;
+import org.junit.jupiter.api.Test;
+import org.mockito.ArgumentCaptor;
+import org.springframework.context.ApplicationEventPublisher;
+import vip.mate.skill.event.SkillRemovedEvent;
+import vip.mate.skill.model.SkillEntity;
+import vip.mate.skill.repository.SkillFileMapper;
+import vip.mate.skill.repository.SkillMapper;
+import vip.mate.skill.runtime.SkillRuntimeService;
+import vip.mate.skill.secret.SkillSecretService;
+import vip.mate.skill.workspace.SkillWorkspaceManager;
+import vip.mate.skill.workspace.SkillWorkspaceProperties;
+
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.mockito.Mockito.mock;
+import static org.mockito.Mockito.verify;
+import static org.mockito.Mockito.when;
+
+/**
+ * Issue #127 — verifies the delete paths publish {@link SkillRemovedEvent}
+ * so the agent-binding listener can scrub {@code mate_agent_skill} orphans.
+ *
+ * Earlier behavior dropped only {@code mate_skill}/{@code mate_skill_file}
+ * /secrets/workspace and left binding rows pointing at a vanished skill id,
+ * which is what users saw as "agent still shows N skills bound".
+ */
+class SkillServiceRemovalEventTest {
+
+ @Test
+ @DisplayName("uninstallSkill publishes SkillRemovedEvent with the row's id and name")
+ void uninstallPublishesEvent() {
+ SkillMapper mapper = mock(SkillMapper.class);
+ SkillFileMapper fileMapper = mock(SkillFileMapper.class);
+ SkillWorkspaceManager workspaceManager = mock(SkillWorkspaceManager.class);
+ SkillWorkspaceProperties workspaceProps = mock(SkillWorkspaceProperties.class);
+ SkillSecretService secretService = mock(SkillSecretService.class);
+ SkillRuntimeService runtimeService = mock(SkillRuntimeService.class);
+ ApplicationEventPublisher publisher = mock(ApplicationEventPublisher.class);
+
+ SkillEntity row = new SkillEntity();
+ row.setId(42L);
+ row.setName("pdf");
+ row.setBuiltin(false);
+ when(mapper.selectById(42L)).thenReturn(row);
+ // Workspace policy other than "archive" keeps the test focused on the
+ // event publish behavior — the archive branch has its own coverage.
+ when(workspaceProps.getDeletePolicy()).thenReturn("purge");
+
+ SkillService service = new SkillService(
+ mapper, fileMapper, workspaceManager, workspaceProps, secretService, publisher);
+ service.setRuntimeService(runtimeService);
+
+ service.uninstallSkill(42L);
+
+ ArgumentCaptor