perf(skills): skip symlink probes beyond discovery depth (#18937)

config/scripts/benchmark-skill-depth.mjs

@@ -0,0 +1,122 @@
+import assert from 'node:assert/strict'
+import { readFileSync } from 'node:fs'
+import * as fs from 'node:fs/promises'
+import Module from 'node:module'
+import { tmpdir } from 'node:os'
+import { join, resolve } from 'node:path'
+import { performance } from 'node:perf_hooks'
+import { build } from 'esbuild'
+
+// Pass a pre-change skill-root-file-walk.ts snapshot as the only argument.
+const baselinePath = process.argv[2]
+const brokenLinks = process.argv.includes('--broken')
+assert.ok(baselinePath, 'Pass a pre-change skill-root-file-walk.ts snapshot.')
+const entry = 'src/main/skills/skill-root-file-walk.ts'
+const baseline = readFileSync(baselinePath, 'utf8')
+assert.notEqual(baseline, readFileSync(entry, 'utf8'), 'Do not compare the source to itself.')
+let statCalls = 0
+
+async function load(useBaseline) {
+  const result = await build({
+    entryPoints: [entry],
+    bundle: true,
+    platform: 'node',
+    format: 'cjs',
+    write: false,
+    logLevel: 'silent',
+    plugins: useBaseline
+      ? [
+          {
+            name: 'baseline-skill-depth',
+            setup(builder) {
+              builder.onLoad({ filter: /skill-root-file-walk\.ts$/ }, () => ({
+                contents: baseline,
+                loader: 'ts'
+              }))
+            }
+          }
+        ]
+      : []
+  })
+  const module = new Module(resolve('skill-depth-benchmark.cjs'))
+  module.paths = Module._nodeModulePaths(process.cwd())
+  const originalRequire = module.require.bind(module)
+  module.require = (name) =>
+    name === 'node:fs/promises'
+      ? {
+          ...fs,
+          stat: (...args) => {
+            statCalls++
+            return fs.stat(...args)
+          }
+        }
+      : originalRequire(name)
+  module._compile(result.outputFiles[0].text, module.id)
+  return module.exports.findSkillFiles
+}
+
+const before = await load(true)
+const after = await load(false)
+const median = (values) => values.sort((a, b) => a - b)[Math.floor(values.length / 2)]
+const temporaryRoot = await fs.mkdtemp(join(tmpdir(), 'orca-skill-depth-benchmark-'))
+try {
+  for (const links of [0, 8, 100, 1000]) {
+    const root = join(temporaryRoot, String(links))
+    const edge = join(root, 'a', 'b', 'c', 'd')
+    const target = join(temporaryRoot, 'target')
+    await fs.mkdir(edge, { recursive: true })
+    await fs.mkdir(target, { recursive: true })
+    await fs.writeFile(join(target, 'SKILL.md'), 'skill')
+    await fs.writeFile(join(edge, 'SKILL.md'), 'edge')
+    for (let index = 0; index < links; index++) {
+      await fs.symlink(
+        brokenLinks ? join(target, 'missing') : target,
+        join(edge, `link${index}`),
+        process.platform === 'win32' ? 'junction' : 'dir'
+      )
+    }
+    for (const depth of [4, 5]) {
+      const timings = { before: [], after: [] }
+      const counts = {}
+      let rows
+      for (let sample = 0; sample < 13; sample++) {
+        const versions =
+          sample % 2
+            ? [
+                ['after', after],
+                ['before', before]
+              ]
+            : [
+                ['before', before],
+                ['after', after]
+              ]
+        for (const [name, walk] of versions) {
+          statCalls = 0
+          const start = performance.now()
+          const result = await walk(root, depth)
+          const elapsed = performance.now() - start
+          if (rows) {
+            assert.deepEqual(result, rows)
+          }
+          rows = result
+          counts[name] = statCalls
+          if (sample >= 2) {
+            timings[name].push(elapsed)
+          }
+        }
+      }
+      console.log(
+        JSON.stringify({
+          links,
+          brokenLinks,
+          depth,
+          statCalls: counts,
+          rows: rows.length,
+          medianMs: { before: median(timings.before), after: median(timings.after) }
+        })
+      )
+    }
+  }
+} finally {
+  await fs.rm(temporaryRoot, { recursive: true, force: true })
+}

src/main/skills/skill-root-file-walk.test.ts

@@ -45,6 +45,34 @@ describe('findSkillFiles', () => {
     expect(found).toEqual([join(root, 'near', 'SKILL.md')])
   })
 
+  it('does not stat directory links beyond the depth bound but still follows in-bound links', async () => {
+    const base = await makeTree()
+    const root = join(base, 'skills')
+    const edge = join(root, 'a', 'b', 'c', 'd')
+    const target = join(base, 'linked')
+    await writeFileAt(join(edge, 'SKILL.md'))
+    await writeFileAt(join(target, 'SKILL.md'))
+    for (let index = 0; index < 32; index += 1) {
+      await symlink(
+        target,
+        join(edge, `link${index.toString().padStart(2, '0')}`),
+        process.platform === 'win32' ? 'junction' : 'dir'
+      )
+    }
+    const statPaths: string[] = []
+    onStat = async (path) => {
+      statPaths.push(path)
+    }
+
+    expect(await findSkillFiles(root, 4)).toEqual([join(edge, 'SKILL.md')])
+    expect(statPaths).toEqual([])
+    expect(await findSkillFiles(root, 5)).toEqual([
+      join(edge, 'SKILL.md'),
+      join(edge, 'link00', 'SKILL.md')
+    ])
+    expect(statPaths).toHaveLength(32)
+  })
+
   it('returns nothing for a missing root rather than throwing', async () => {
     expect(await findSkillFiles(join(await makeTree(), 'absent'), 4)).toEqual([])
   })

src/main/skills/skill-root-file-walk.ts

@@ -43,9 +43,6 @@ export async function findSkillFiles(
     // indistinguishable from a genuinely small root, and a caller that cached it
     // would publish "these skills no longer exist".
     signal?.throwIfAborted()
-    if (!isWithinDepth(rootPath, dirPath, maxDepth)) {
-      return
-    }
     let resolvedDirPath: string
     try {
       resolvedDirPath = await realpath(dirPath)
@@ -61,6 +58,8 @@ export async function findSkillFiles(
     if (!entries) {
       return
     }
+    // Directory entry names add one segment, so siblings share the depth verdict.
+    let childrenWithinDepth: boolean | undefined
     for (const entry of entries) {
       signal?.throwIfAborted()
       // Why: a staged sibling sits directly in a scanned root, so without this a
@@ -86,10 +85,15 @@ export async function findSkillFiles(
         continue
       }
       if (entry.isDirectory()) {
-        await visit(entryPath)
+        if ((childrenWithinDepth ??= isWithinDepth(rootPath, entryPath, maxDepth))) {
+          await visit(entryPath)
+        }
         continue
       }
-      if (entry.isSymbolicLink()) {
+      if (
+        entry.isSymbolicLink() &&
+        (childrenWithinDepth ??= isWithinDepth(rootPath, entryPath, maxDepth))
+      ) {
         // Why: users commonly symlink agent skill dirs across providers; follow
         // directory links but guard by realpath so recursive links cannot loop.
         let linksToDirectory = false