Harden managed skill update stabilization

src/main/runtime/rpc/methods/skills.test.ts

@@ -3,6 +3,7 @@ import { RpcDispatcher } from '../dispatcher'
 import type { RpcRequest, RpcResponse } from '../core'
 import { SKILL_METHODS } from './skills'
 import {
+  onManagedSkillEvent,
   publishManagedSkillFallback,
   publishManagedSkillUpdated
 } from '../../../skills/managed-skill-events'
@@ -129,4 +130,34 @@ describe('skills runtime RPC methods', () => {
       result: { type: 'end' }
     })
   })
+
+  it('continues publishing managed skill events when one listener throws', () => {
+    const received: unknown[] = []
+    const unsubscribeThrowing = onManagedSkillEvent(() => {
+      throw new Error('listener failed')
+    })
+    const unsubscribeReceiving = onManagedSkillEvent((event) => {
+      received.push(event)
+    })
+    const fallbackEvent = {
+      status: 'fallback',
+      skillName: 'orca-linear',
+      context: 'linear-worktree',
+      runtime: 'host',
+      scope: 'missing',
+      reason: 'missing-install',
+      uiKey: 'host::orca-linear:linear-worktree',
+      message: 'Install orca-linear.',
+      request: { skillName: 'orca-linear', context: 'linear-worktree' }
+    } satisfies ManagedAgentSkillFallback
+
+    try {
+      publishManagedSkillFallback(fallbackEvent)
+    } finally {
+      unsubscribeThrowing()
+      unsubscribeReceiving()
+    }
+
+    expect(received).toEqual([{ type: 'fallback', event: fallbackEvent }])
+  })
 })

src/main/skills/managed-skill-events.ts

@@ -34,6 +34,10 @@ export function publishManagedSkillUpdated(result: ManagedAgentSkillEnsureResult
 
 function publishManagedSkillEvent(event: ManagedSkillEvent): void {
   for (const listener of managedSkillEventListeners) {
-    listener(event)
+    try {
+      listener(event)
+    } catch {
+      // Why: one broken stream subscriber should not interrupt other skill update listeners.
+    }
   }
 }

src/main/skills/managed-skill-test-fixtures.ts

@@ -1,5 +1,5 @@
 import { homedir } from 'node:os'
-import { join } from 'node:path'
+import { join, sep } from 'node:path'
 import { ORCHESTRATION_SKILL_NAME } from '../../shared/agent-feature-install-commands'
 import type {
   DiscoveredSkill,
@@ -9,11 +9,16 @@ import type {
 } from '../../shared/skills'
 
 export const TEST_MANAGED_HOME_ROOT = join(homedir(), '.agents', 'skills')
+export const TEST_ALICE_HOME = join(sep, 'home', 'alice')
+export const TEST_CENTRAL_SKILLS_ROOT = join(sep, 'mnt', 'central', 'skills')
+export const TEST_WORKSPACE_ROOT = join(sep, 'workspace', 'current')
+export const TEST_OTHER_WORKSPACE_ROOT = join(sep, 'workspace', 'other')
+export const TEST_WORKSPACE_AGENT_SKILLS_ROOT = join(TEST_WORKSPACE_ROOT, '.agents', 'skills')
 
 export const orchestrationRequest: ManagedAgentSkillEnsureRequest = {
   skillName: ORCHESTRATION_SKILL_NAME,
   context: 'agent-orchestration',
-  discoveryTarget: { runtime: 'host', projectRootPath: '/workspace/current' }
+  discoveryTarget: { runtime: 'host', projectRootPath: TEST_WORKSPACE_ROOT }
 }
 
 export function discoveredSkill(
@@ -21,6 +26,7 @@ export function discoveredSkill(
 ): DiscoveredSkill {
   const rootPath = patch.rootPath ?? TEST_MANAGED_HOME_ROOT
   const directoryPath = patch.directoryPath ?? join(rootPath, patch.name)
+  const realDirectoryPath = patch.realDirectoryPath ?? directoryPath
   return {
     id: patch.id ?? `${patch.sourceKind}-${patch.name}`,
     name: patch.name,
@@ -30,10 +36,10 @@ export function discoveredSkill(
     sourceLabel: patch.sourceLabel ?? patch.sourceKind,
     rootPath,
     directoryPath,
-    realDirectoryPath: patch.realDirectoryPath ?? directoryPath,
+    realDirectoryPath,
     directoryIsSymlink: patch.directoryIsSymlink ?? false,
     skillFilePath: patch.skillFilePath ?? join(directoryPath, 'SKILL.md'),
-    realSkillFilePath: patch.realSkillFilePath ?? join(directoryPath, 'SKILL.md'),
+    realSkillFilePath: patch.realSkillFilePath ?? join(realDirectoryPath, 'SKILL.md'),
     skillFileIsSymlink: patch.skillFileIsSymlink ?? false,
     installed: patch.installed ?? true,
     fileCount: patch.fileCount ?? 1,

src/main/skills/managed-skill-update-runner.test.ts

@@ -1,8 +1,14 @@
 import { EventEmitter } from 'node:events'
 import { spawn } from 'node:child_process'
+import { join, sep } from 'node:path'
 import { afterEach, describe, expect, it, vi } from 'vitest'
 import { ORCHESTRATION_SKILL_NAME } from '../../shared/agent-feature-install-commands'
+import { homeDiscovery, lockfile, orchestrationRequest } from './managed-skill-test-fixtures'
 import { createManagedSkillUpdateRunner } from './managed-skill-update-runner'
+import {
+  abortManagedSkillUpdateProcesses,
+  ManagedSkillUpdateCoordinator
+} from './managed-skill-updates'
 
 vi.mock('node:child_process', () => ({
   spawn: vi.fn()
@@ -17,6 +23,8 @@ class FakeChildProcess extends EventEmitter {
   }
 }
 
+const TEST_ALICE_HOME = join(sep, 'home', 'alice')
+
 describe('createManagedSkillUpdateRunner', () => {
   afterEach(() => {
     vi.useRealTimers()
@@ -49,10 +57,22 @@ describe('createManagedSkillUpdateRunner', () => {
     expect(child.kill).toHaveBeenCalledTimes(1)
   })
 
+  it('does not spawn when the shutdown signal already aborted', async () => {
+    const controller = new AbortController()
+    controller.abort()
+    const runner = createManagedSkillUpdateRunner({ signal: controller.signal, timeoutMs: 1_000 })
+
+    await expect(runner(ORCHESTRATION_SKILL_NAME)).resolves.toEqual({
+      status: 'failure',
+      error: 'aborted'
+    })
+    expect(spawn).not.toHaveBeenCalled()
+  })
+
   it('spawns the single-skill global update command without a shell from a neutral cwd', async () => {
     const child = new FakeChildProcess()
     vi.mocked(spawn).mockReturnValue(child as never)
-    const runner = createManagedSkillUpdateRunner({ cwd: '/home/alice', timeoutMs: 1_000 })
+    const runner = createManagedSkillUpdateRunner({ cwd: TEST_ALICE_HOME, timeoutMs: 1_000 })
 
     const resultPromise = runner(ORCHESTRATION_SKILL_NAME)
     child.emit('close', 0)
@@ -62,7 +82,7 @@ describe('createManagedSkillUpdateRunner', () => {
       'npx',
       ['--yes', 'skills', 'update', 'orchestration', '--global', '--yes'],
       {
-        cwd: '/home/alice',
+        cwd: TEST_ALICE_HOME,
         shell: false,
         stdio: 'ignore',
         windowsHide: true
@@ -93,4 +113,25 @@ describe('createManagedSkillUpdateRunner', () => {
     expect(killer.unref).toHaveBeenCalledTimes(1)
     platform.mockRestore()
   })
+
+  it('allows future default coordinators to update after aborting current update processes', async () => {
+    const child = new FakeChildProcess()
+    vi.mocked(spawn).mockReturnValue(child as never)
+    abortManagedSkillUpdateProcesses()
+    const coordinator = new ManagedSkillUpdateCoordinator({
+      backgroundUpdatesEnabled: () => true,
+      discoverHostSkills: async () => homeDiscovery(),
+      readTextFile: async () => lockfile(ORCHESTRATION_SKILL_NAME, 'same-hash')
+    })
+
+    const resultPromise = coordinator.ensureManagedReady(orchestrationRequest)
+    await vi.waitFor(() => expect(spawn).toHaveBeenCalled())
+    child.emit('close', 0)
+
+    await expect(resultPromise).resolves.toMatchObject({
+      status: 'ready',
+      skillName: ORCHESTRATION_SKILL_NAME,
+      context: 'agent-orchestration'
+    })
+  })
 })

src/main/skills/managed-skill-update-runner.ts

@@ -41,6 +41,10 @@ async function runManagedSkillUpdate(
   cwd: string,
   signal: AbortSignal | undefined
 ): Promise<ManagedSkillUpdateRunnerResult> {
+  if (signal?.aborted) {
+    return { status: 'failure', error: 'aborted' }
+  }
+
   const command = buildManagedSkillUpdateCommand(skillName)
   return new Promise((resolve) => {
     let settled = false
@@ -73,10 +77,6 @@ async function runManagedSkillUpdate(
       killChildTree()
       finish({ status: 'timeout' })
     }, timeoutMs)
-    if (signal?.aborted) {
-      onAbort()
-      return
-    }
     signal?.addEventListener('abort', onAbort, { once: true })
 
     child.once('error', (error) => {

src/main/skills/managed-skill-updates-safety.test.ts

@@ -1,4 +1,5 @@
 import { describe, expect, it, vi } from 'vitest'
+import { join } from 'node:path'
 import {
   ORCA_LINEAR_SKILL_NAME,
   ORCHESTRATION_SKILL_NAME
@@ -6,6 +7,10 @@ import {
 import type { ManagedAgentSkillEnsureRequest, ManagedAgentSkillName } from '../../shared/skills'
 import {
   TEST_MANAGED_HOME_ROOT,
+  TEST_ALICE_HOME,
+  TEST_CENTRAL_SKILLS_ROOT,
+  TEST_OTHER_WORKSPACE_ROOT,
+  TEST_WORKSPACE_AGENT_SKILLS_ROOT,
   discoveredSkill,
   discovery,
   homeDiscovery,
@@ -24,8 +29,8 @@ describe('ManagedSkillUpdateCoordinator safety cases', () => {
           discoveredSkill({
             name: ORCHESTRATION_SKILL_NAME,
             sourceKind: 'repo',
-            rootPath: '/workspace/current/.agents/skills',
-            directoryPath: '/workspace/current/.agents/skills/orchestration'
+            rootPath: TEST_WORKSPACE_AGENT_SKILLS_ROOT,
+            directoryPath: join(TEST_WORKSPACE_AGENT_SKILLS_ROOT, 'orchestration')
           })
         ],
         reason: 'project-install'
@@ -36,8 +41,8 @@ describe('ManagedSkillUpdateCoordinator safety cases', () => {
           discoveredSkill({
             name: ORCHESTRATION_SKILL_NAME,
             sourceKind: 'repo',
-            rootPath: '/workspace/current/.agents/skills',
-            directoryPath: '/workspace/current/.agents/skills/orchestration'
+            rootPath: TEST_WORKSPACE_AGENT_SKILLS_ROOT,
+            directoryPath: join(TEST_WORKSPACE_AGENT_SKILLS_ROOT, 'orchestration')
           })
         ],
         reason: 'ambiguous-install'
@@ -65,10 +70,10 @@ describe('ManagedSkillUpdateCoordinator safety cases', () => {
           discoveredSkill({
             name: ORCHESTRATION_SKILL_NAME,
             sourceKind: 'home',
-            directoryPath: '/home/alice/.agents/skills/orchestration',
-            realDirectoryPath: '/mnt/central/skills/orchestration',
-            skillFilePath: '/home/alice/.agents/skills/orchestration/SKILL.md',
-            realSkillFilePath: '/mnt/central/skills/orchestration/SKILL.md'
+            directoryPath: join(TEST_ALICE_HOME, '.agents', 'skills', 'orchestration'),
+            realDirectoryPath: join(TEST_CENTRAL_SKILLS_ROOT, 'orchestration'),
+            skillFilePath: join(TEST_ALICE_HOME, '.agents', 'skills', 'orchestration', 'SKILL.md'),
+            realSkillFilePath: join(TEST_CENTRAL_SKILLS_ROOT, 'orchestration', 'SKILL.md')
           })
         ],
         reason: 'symlinked-global-install'
@@ -91,15 +96,15 @@ describe('ManagedSkillUpdateCoordinator safety cases', () => {
   it('does not treat Codex or Claude home skills as managed global installs', async () => {
     const updateRunner = vi.fn(async () => ({ status: 'success' as const }))
     const coordinator = new ManagedSkillUpdateCoordinator({
-      homeDir: () => '/home/alice',
+      homeDir: () => TEST_ALICE_HOME,
       discoverHostSkills: async () =>
         discovery([
           discoveredSkill({
             name: ORCHESTRATION_SKILL_NAME,
             sourceKind: 'home',
             providers: ['codex'],
-            rootPath: '/home/alice/.codex/skills',
-            directoryPath: '/home/alice/.codex/skills/orchestration'
+            rootPath: join(TEST_ALICE_HOME, '.codex', 'skills'),
+            directoryPath: join(TEST_ALICE_HOME, '.codex', 'skills', 'orchestration')
           })
         ]),
       readTextFile: async () => lockfile(ORCHESTRATION_SKILL_NAME, 'old-hash'),
@@ -124,8 +129,8 @@ describe('ManagedSkillUpdateCoordinator safety cases', () => {
         discoveredSkill({
           name: ORCHESTRATION_SKILL_NAME,
           sourceKind: 'repo',
-          rootPath: '/workspace/current/.agents/skills',
-          directoryPath: '/workspace/current/.agents/skills/orchestration'
+          rootPath: TEST_WORKSPACE_AGENT_SKILLS_ROOT,
+          directoryPath: join(TEST_WORKSPACE_AGENT_SKILLS_ROOT, 'orchestration')
         })
       ])
     )
@@ -140,7 +145,7 @@ describe('ManagedSkillUpdateCoordinator safety cases', () => {
       coordinator.ensureManagedReady(orchestrationRequest),
       coordinator.ensureManagedReady({
         ...orchestrationRequest,
-        discoveryTarget: { runtime: 'host', projectRootPath: '/workspace/other' }
+        discoveryTarget: { runtime: 'host', projectRootPath: TEST_OTHER_WORKSPACE_ROOT }
       })
     ])
 

src/main/skills/managed-skill-updates.test.ts

@@ -1,6 +1,11 @@
 import { describe, expect, it, vi } from 'vitest'
 import { ORCHESTRATION_SKILL_NAME } from '../../shared/agent-feature-install-commands'
-import { homeDiscovery, lockfile, orchestrationRequest } from './managed-skill-test-fixtures'
+import {
+  TEST_OTHER_WORKSPACE_ROOT,
+  homeDiscovery,
+  lockfile,
+  orchestrationRequest
+} from './managed-skill-test-fixtures'
 import { ManagedSkillUpdateCoordinator } from './managed-skill-updates'
 
 describe('ManagedSkillUpdateCoordinator', () => {
@@ -113,7 +118,7 @@ describe('ManagedSkillUpdateCoordinator', () => {
     const first = await coordinator.ensureManagedReady(orchestrationRequest)
     const second = await coordinator.ensureManagedReady({
       ...orchestrationRequest,
-      discoveryTarget: { runtime: 'host', projectRootPath: '/workspace/other' }
+      discoveryTarget: { runtime: 'host', projectRootPath: TEST_OTHER_WORKSPACE_ROOT }
     })
 
     expect(first.status).toBe('updated')

src/main/skills/managed-skill-updates.ts

@@ -43,10 +43,12 @@ type ManagedSkillCoordinatorDeps = {
 }
 
 const DEFAULT_COOLDOWN_MS = 60_000
-const defaultUpdateAbortController = new AbortController()
+let defaultUpdateAbortController = new AbortController()
 
 export function abortManagedSkillUpdateProcesses(): void {
   defaultUpdateAbortController.abort()
+  // Why: AbortSignal is one-shot; future coordinators need a fresh shutdown signal.
+  defaultUpdateAbortController = new AbortController()
 }
 
 export class ManagedSkillUpdateCoordinator {