fix(cli): harden doctor scan and inventory writes

Protect doctor metadata scanning from symlinked agent, skill, and .skillhub directories, and make inventory mutations use locked atomic writes with stale lock recovery.

cli/src/services/doctor-service.ts

@@ -1,4 +1,4 @@
-import { readdir, writeFile, readFile } from 'node:fs/promises'
+import { lstat, readdir, writeFile, readFile } from 'node:fs/promises'
 import { join } from 'node:path'
 import { CliError } from '../shared/errors'
 import { EXIT } from '../shared/constants'
@@ -98,7 +98,18 @@ async function scanMetadata(cwd: string, skipped: DoctorResult['skipped']): Prom
 
   for (const dirName of topEntries) {
     if (!dirName.startsWith('.')) continue
-    const skillsDir = join(cwd, dirName, 'skills')
+    const agentDir = join(cwd, dirName)
+    try {
+      const st = await lstat(agentDir)
+      if (st.isSymbolicLink() || !st.isDirectory()) {
+        skipped.push({ path: agentDir, reason: 'not a regular directory' })
+        continue
+      }
+    } catch {
+      skipped.push({ path: agentDir, reason: 'cannot stat' })
+      continue
+    }
+    const skillsDir = join(agentDir, 'skills')
     let slugDirs: string[]
     try {
       slugDirs = await readdir(skillsDir)
@@ -107,17 +118,39 @@ async function scanMetadata(cwd: string, skipped: DoctorResult['skipped']): Prom
     }
 
     for (const slug of slugDirs) {
-      const metadataPath = join(skillsDir, slug, '.skillhub', 'metadata.json')
+      const slugPath = join(skillsDir, slug)
+      try {
+        const st = await lstat(slugPath)
+        if (st.isSymbolicLink() || !st.isDirectory()) {
+          skipped.push({ path: slugPath, reason: 'not a regular directory' })
+          continue
+        }
+      } catch {
+        skipped.push({ path: slugPath, reason: 'cannot stat' })
+        continue
+      }
+      const skillhubDir = join(slugPath, '.skillhub')
+      try {
+        const skillhubSt = await lstat(skillhubDir)
+        if (skillhubSt.isSymbolicLink() || !skillhubSt.isDirectory()) {
+          skipped.push({ path: slugPath, reason: '.skillhub is not a regular directory' })
+          continue
+        }
+      } catch {
+        skipped.push({ path: slugPath, reason: 'no .skillhub directory' })
+        continue
+      }
+      const metadataPath = join(skillhubDir, 'metadata.json')
       try {
         const content = await readFile(metadataPath, 'utf-8')
         const metadata = JSON.parse(content) as MetadataJson
         if (!metadata.registry || !metadata.namespace || !metadata.slug || !metadata.version || !metadata.agent || !metadata.installedAt) {
-          skipped.push({ path: join(skillsDir, slug), reason: 'incomplete metadata' })
+          skipped.push({ path: slugPath, reason: 'incomplete metadata' })
           continue
         }
-        results.push({ metadata, installDir: join(skillsDir, slug) })
+        results.push({ metadata, installDir: slugPath })
       } catch {
-        skipped.push({ path: join(skillsDir, slug), reason: 'no .skillhub/metadata.json' })
+        skipped.push({ path: slugPath, reason: 'no .skillhub/metadata.json' })
       }
     }
   }

cli/src/stores/inventory-store.ts

@@ -1,4 +1,4 @@
-import { readFile, rename, writeFile } from 'node:fs/promises'
+import { open, readFile, rename, rm, writeFile } from 'node:fs/promises'
 import { dirname } from 'node:path'
 import { joinPath, userStateDir, ensureDir, pathExists } from '../platform/paths'
 
@@ -42,10 +42,74 @@ export class InventoryStore {
     await ensureDir(dirname(this.path))
     const payload = JSON.stringify(inventory, null, 2)
     JSON.parse(payload)
+
+    const lockPath = `${this.path}.lock`
     const tmpPath = `${this.path}.${process.pid}.${Date.now()}.tmp`
-    await writeFile(tmpPath, payload)
-    JSON.parse(await readFile(tmpPath, 'utf-8'))
-    await rename(tmpPath, this.path)
+
+    let lockHandle: Awaited<ReturnType<typeof open>> | null = null
+    try {
+      // Acquire exclusive lock with retry and stale lock detection
+      lockHandle = await this.acquireLock(lockPath)
+
+      await writeFile(tmpPath, payload)
+      JSON.parse(await readFile(tmpPath, 'utf-8'))
+      await rename(tmpPath, this.path)
+    } finally {
+      // Clean up temp file if it still exists
+      await rm(tmpPath, { force: true }).catch(() => {})
+
+      // Release lock
+      if (lockHandle) {
+        await lockHandle.close().catch(() => {})
+        await rm(lockPath, { force: true }).catch(() => {})
+      }
+    }
+  }
+
+  private async acquireLock(lockPath: string, maxRetries = 10, retryDelayMs = 100): Promise<Awaited<ReturnType<typeof open>>> {
+    for (let attempt = 0; attempt < maxRetries; attempt++) {
+      try {
+        // Try to create lock file with PID and timestamp
+        const lockHandle = await open(lockPath, 'wx')
+        const lockData = JSON.stringify({ pid: process.pid, timestamp: Date.now() })
+        await writeFile(lockPath, lockData)
+        return lockHandle
+      } catch (err: any) {
+        if (err.code !== 'EEXIST') throw err
+
+        // Lock exists, check if it's stale (older than 30 seconds)
+        // 30s threshold chosen to balance between:
+        // - Allowing slow operations to complete (e.g., large inventory writes)
+        // - Recovering quickly from crashed processes
+        try {
+          const lockContent = await readFile(lockPath, 'utf-8')
+          const lockData = JSON.parse(lockContent) as { pid: number; timestamp: number }
+          const ageMs = Date.now() - lockData.timestamp
+
+          if (ageMs > 30000) {
+            // Stale lock detected - verify the process is actually dead
+            try {
+              // process.kill(pid, 0) throws if process doesn't exist
+              process.kill(lockData.pid, 0)
+              // Process still alive, wait and retry
+            } catch {
+              // Process is dead, safe to remove stale lock
+              await rm(lockPath, { force: true }).catch(() => {})
+              continue
+            }
+          }
+        } catch {
+          // Lock file disappeared or corrupted, retry
+          continue
+        }
+
+        // Lock is held by another active process, wait and retry with exponential backoff
+        if (attempt < maxRetries - 1) {
+          await new Promise(resolve => setTimeout(resolve, retryDelayMs * Math.pow(2, attempt)))
+        }
+      }
+    }
+    throw new Error(`Failed to acquire lock after ${maxRetries} attempts`)
   }
 
   async upsertTarget(
@@ -70,7 +134,7 @@ export class InventoryStore {
     } else {
       item.targets.push(target)
     }
-    await this.write(inventory)
+    await this.writeAtomic(inventory)
   }
 
   async removeTarget(registry: string, namespace: string, slug: string, installDir: string): Promise<boolean> {
@@ -83,7 +147,7 @@ export class InventoryStore {
     if (item.targets.length === 0) {
       inventory.items = inventory.items.filter(i => i !== item)
     }
-    await this.write(inventory)
+    await this.writeAtomic(inventory)
     return true
   }
 
@@ -97,7 +161,7 @@ export class InventoryStore {
     }
     if (removed > 0) {
       inventory.items = inventory.items.filter(item => item.targets.length > 0)
-      await this.write(inventory)
+      await this.writeAtomic(inventory)
     }
     return removed
   }

cli/test/unit/services/doctor-service.test.ts

@@ -1,5 +1,5 @@
 import { describe, expect, test } from 'bun:test'
-import { mkdtemp, mkdir, writeFile } from 'node:fs/promises'
+import { mkdtemp, mkdir, writeFile, symlink } from 'node:fs/promises'
 import { tmpdir } from 'node:os'
 import { join } from 'node:path'
 import { runDoctor } from '../../../src/services/doctor-service'
@@ -42,7 +42,7 @@ describe('doctor-service', () => {
     const result = await runDoctor(cwd, home)
     expect(result.itemsRestored).toBe(0)
     expect(result.skipped).toHaveLength(1)
-    expect(result.skipped[0]!.reason).toBe('no .skillhub/metadata.json')
+    expect(result.skipped[0]!.reason).toBe('no .skillhub directory')
   })
 
   test('detects version conflicts', async () => {
@@ -64,4 +64,61 @@ describe('doctor-service', () => {
     expect(result.conflicts[0]!.versions).toContain('1.0.0')
     expect(result.conflicts[0]!.versions).toContain('2.0.0')
   })
+
+  test('skips symlinked skill directories', async () => {
+    const cwd = await mkdtemp(join(tmpdir(), 'doctor-test-'))
+    const home = await mkdtemp(join(tmpdir(), 'doctor-home-'))
+
+    await setupSkillDir(cwd, '.codex', 'real-skill', {
+      registry: 'https://skill.xfyun.cn', namespace: 'global', slug: 'real-skill',
+      version: '1.0.0', agent: 'codex', installedAt: '2026-04-20T12:00:00Z'
+    })
+
+    const skillsDir = join(cwd, '.codex', 'skills')
+    const realDir = join(skillsDir, 'real-skill')
+    await symlink(realDir, join(skillsDir, 'symlink-skill'))
+
+    const result = await runDoctor(cwd, home)
+    expect(result.itemsRestored).toBe(1)
+    expect(result.skipped.some(s => s.reason === 'not a regular directory')).toBe(true)
+  })
+
+  test('skips symlinked agent directories', async () => {
+    const cwd = await mkdtemp(join(tmpdir(), 'doctor-test-'))
+    const home = await mkdtemp(join(tmpdir(), 'doctor-home-'))
+
+    await setupSkillDir(cwd, '.codex', 'pdf-parser', {
+      registry: 'https://skill.xfyun.cn', namespace: 'global', slug: 'pdf-parser',
+      version: '1.0.0', agent: 'codex', installedAt: '2026-04-20T12:00:00Z'
+    })
+
+    await symlink(join(cwd, '.codex'), join(cwd, '.fake-agent'))
+
+    const result = await runDoctor(cwd, home)
+    expect(result.itemsRestored).toBe(1)
+    expect(result.targetsRestored).toBe(1)
+  })
+
+  test('skips symlinked .skillhub directories', async () => {
+    const cwd = await mkdtemp(join(tmpdir(), 'doctor-test-'))
+    const home = await mkdtemp(join(tmpdir(), 'doctor-home-'))
+
+    const skillDir = join(cwd, '.codex', 'skills', 'evil-skill')
+    await mkdir(skillDir, { recursive: true })
+
+    const realMetaDir = await mkdtemp(join(tmpdir(), 'real-meta-'))
+    await writeFile(join(realMetaDir, 'metadata.json'), JSON.stringify({
+      registry: 'https://skill.xfyun.cn',
+      namespace: 'global',
+      slug: 'evil-skill',
+      version: '1.0.0',
+      agent: 'codex',
+      installedAt: '2026-04-20T12:00:00Z'
+    }))
+    await symlink(realMetaDir, join(skillDir, '.skillhub'))
+
+    const result = await runDoctor(cwd, home)
+    expect(result.itemsRestored).toBe(0)
+    expect(result.skipped.some(s => s.reason === '.skillhub is not a regular directory')).toBe(true)
+  })
 })