chore(core): remove the unused trusted hooks manager and implement unregisterSkillHooks

eslint.legacy-filenames.mjs

@@ -402,7 +402,6 @@ export const legacyFilenames = [
   'truncatedDiffPreview',
   'trustCommand',
   'trustedFolders',
-  'trustedHooks',
   'TurnBuffer',
   'uiTelemetry',
   'updateCheck',

packages/cli/src/agent-view/supervisor-store.ts

@@ -698,7 +698,7 @@ async function writeJsonFile(
   await fs.mkdir(path.dirname(filePath), { recursive: true });
   // noFollow: these files carry the supervisor and worker auth tokens;
   // refuse to write through a pre-placed symlink at the store path, matching
-  // the other credential write sites (trustedHooks, file-token-storage).
+  // the other credential write site (file-token-storage).
   await atomicWriteFile(filePath, `${JSON.stringify(value, null, 2)}\n`, {
     mode: 0o600,
     forceMode: true,

packages/core/src/hooks/hookRegistry.test.ts

@@ -10,14 +10,6 @@ import { HookRegistry } from './hookRegistry.js';
 import { HookEventName, HooksConfigSource, HookType } from './types.js';
 import type { HookConfig } from './types.js';
 
-// Mock TrustedHooksManager
-vi.mock('./trustedHooks.js', () => ({
-  TrustedHooksManager: vi.fn().mockImplementation(() => ({
-    getUntrustedHooks: vi.fn().mockReturnValue([]),
-    trustHooks: vi.fn(),
-  })),
-}));
-
 describe('HookRegistry', () => {
   let mockConfig: HookRegistryConfig;
   let mockFeedbackEmitter: FeedbackEmitter;

packages/core/src/hooks/registerSkillHooks.test.ts

@@ -5,7 +5,10 @@
  */
 
 import { describe, it, expect, beforeEach } from 'vitest';
-import { registerSkillHooks } from './registerSkillHooks.js';
+import {
+  registerSkillHooks,
+  unregisterSkillHooks,
+} from './registerSkillHooks.js';
 import { SessionHooksManager } from './sessionHooksManager.js';
 import { HookEventName, HookType } from './types.js';
 import type { SkillConfig } from '../skills/types.js';
@@ -412,4 +415,50 @@ describe('registerSkillHooks — the trust gate travels with the entry', () => {
     const [entry] = manager.getHooksForEvent('s1', HookEventName.PreToolUse);
     expect(entry.trustGated).toBeUndefined();
   });
+
+  describe('unregisterSkillHooks', () => {
+    const skillWithRoot = (name: string, root?: string): SkillConfig => ({
+      name,
+      description: name,
+      level: 'user',
+      filePath: `/skills/${name}/SKILL.md`,
+      ...(root ? { skillRoot: root } : {}),
+      body: '',
+      hooks: {
+        [HookEventName.PreToolUse]: [
+          {
+            matcher: 'Bash',
+            hooks: [{ type: HookType.Command, command: `echo ${name}` }],
+          },
+        ],
+      },
+    });
+
+    it('removes only the hooks the skill registered', () => {
+      const manager = new SessionHooksManager();
+      const skillA = skillWithRoot('a', '/skills/a');
+      const skillB = skillWithRoot('b', '/skills/b');
+      registerSkillHooks(manager, 's1', skillA);
+      registerSkillHooks(manager, 's1', skillB);
+
+      expect(unregisterSkillHooks(manager, 's1', skillA)).toBe(1);
+      expect(
+        manager
+          .getHooksForEvent('s1', HookEventName.PreToolUse)
+          .map((entry) => entry.skillRoot),
+      ).toEqual(['/skills/b']);
+      expect(registerSkillHooks(manager, 's1', skillA)).toBe(1);
+    });
+
+    it('removes nothing for a skill without a root directory', () => {
+      const manager = new SessionHooksManager();
+      const rootless = skillWithRoot('rootless');
+      registerSkillHooks(manager, 's1', rootless);
+
+      expect(unregisterSkillHooks(manager, 's1', rootless)).toBe(0);
+      expect(
+        manager.getHooksForEvent('s1', HookEventName.PreToolUse),
+      ).toHaveLength(1);
+    });
+  });
 });

packages/core/src/hooks/registerSkillHooks.ts

@@ -157,13 +157,13 @@ function prepareHookConfig(
 }
 
 /**
- * Unregisters all hooks from a skill.
+ * Unregisters the session hooks a skill registered, identified by the skill's
+ * root directory, and returns how many were removed.
  *
- * Note: This is typically not needed as session hooks are cleared
- * when the session ends. However, it can be useful for cleanup
- * in certain scenarios. Folder-trust revocation does not go through it:
- * a project skill's hooks are registered trust-gated and re-checked at
- * fire time, so no per-skill tracking is needed to silence them.
+ * A skill without a root directory returns 0, because its hooks cannot be
+ * told apart from another skill's. Folder-trust revocation does not go
+ * through this: a project skill's hooks are registered trust-gated and
+ * re-checked at fire time.
  *
  * @param sessionHooksManager - The session hooks manager instance
  * @param sessionId - The current session ID
@@ -175,15 +175,24 @@ export function unregisterSkillHooks(
   sessionId: string,
   skill: SkillConfig,
 ): number {
-  if (!skill.hooks) {
+  if (!skill.hooks || !skill.skillRoot) {
     return 0;
   }
 
-  // Note: Current implementation doesn't track hook IDs per skill
-  // Session hooks are cleared when session ends
-  debugLogger.debug(
-    `Skill hooks for '${skill.name}' will be cleared with session`,
-  );
+  let removed = 0;
+  for (const entry of sessionHooksManager.getAllSessionHooks(sessionId)) {
+    if (
+      entry.skillRoot === skill.skillRoot &&
+      sessionHooksManager.removeHook(sessionId, entry.hookId)
+    ) {
+      removed++;
+    }
+  }
 
-  return 0;
+  if (removed > 0) {
+    debugLogger.debug(
+      `Unregistered ${removed} hooks from skill '${skill.name}'`,
+    );
+  }
+  return removed;
 }