fix(readiness-core): don't let an unreadable skills path abort the whole scan (#525)

scanSkillDirectories() only guarded the readdir() call with a prior
fileExists() check, which just proves the path existed at stat() time
-- it doesn't prove readdir() will succeed on it. A skills path that
exists but isn't actually a directory (renamed, a broken checkout, a
stray file artifact) throws ENOTDIR; an unreadable or permission-
denied directory throws EPERM; a path removed between the two calls
throws ENOENT. All three used to propagate out of this function
uncaught, which aborts the entire audit run in loop-audit and
goal-audit (both call this via findSkills) instead of just skipping
that one broken path and scanning the rest.

tools/mcp-server/src/resolver.ts's listSkills() already handles the
identical shape of problem correctly, wrapping its readdir() in a
try/catch with the same fileExists()-then-readdir() structure. Applied
the same fix here.

Test plan: added a regression test where .claude/skills exists as a
file instead of a directory, asserting the scan completes and still
finds a valid skill under a sibling path instead of throwing. Full
clean rebuild + npm test: 6/6 passing.

Co-authored-by: Claude Sonnet 5 <noreply@anthropic.com>

Author: KhaiTrang1995

tools/readiness-core/src/index.ts

@@ -39,10 +39,19 @@ export async function scanSkillDirectories(root: string): Promise<string[]> {
   const foundSet = new Set<string>();
   for (const dir of dirs) {
     if (!(await fileExists(dir))) continue;
-    const entries = await readdir(dir, { withFileTypes: true });
-    for (const e of entries) {
-      if (e.isDirectory()) foundSet.add(e.name);
-      if (e.isFile() && e.name === 'SKILL.md') foundSet.add('root-skill');
+    try {
+      const entries = await readdir(dir, { withFileTypes: true });
+      for (const e of entries) {
+        if (e.isDirectory()) foundSet.add(e.name);
+        if (e.isFile() && e.name === 'SKILL.md') foundSet.add('root-skill');
+      }
+    } catch {
+      // fileExists only proves the path existed at stat time -- it can still
+      // turn out to not be a directory (ENOTDIR), be unreadable (EPERM), or
+      // be removed between the two calls (ENOENT/TOCTOU). Any of those used
+      // to propagate out of this function uncaught, aborting the whole
+      // audit run in loop-audit/goal-audit instead of just skipping this one
+      // path. Matches tools/mcp-server/src/resolver.ts's listSkills().
     }
   }
   return [...foundSet];

tools/readiness-core/test/index.test.mjs

@@ -74,3 +74,21 @@ test('scanSkillDirectories finds skills in target directory', async () => {
 
   await fs.rm(fixtureDir, { recursive: true, force: true });
 });
+
+test('scanSkillDirectories skips a skills path that is not a directory instead of throwing', async () => {
+  // fileExists() only proves the path existed at stat() time -- it doesn't
+  // prove readdir() will succeed on it. A `skills` path that's actually a
+  // file (renamed, a broken checkout, a stray artifact) used to crash the
+  // whole scan with an uncaught ENOTDIR, aborting the entire audit run in
+  // loop-audit/goal-audit instead of just skipping this one path.
+  const fixtureDir = path.join(process.cwd(), '.test-fixture-not-a-dir');
+  await fs.rm(fixtureDir, { recursive: true, force: true });
+  await fs.mkdir(path.join(fixtureDir, '.claude'), { recursive: true });
+  await fs.writeFile(path.join(fixtureDir, '.claude', 'skills'), 'not actually a directory');
+  await fs.mkdir(path.join(fixtureDir, 'skills', 'bar'), { recursive: true });
+
+  const skills = await scanSkillDirectories(fixtureDir);
+  assert.ok(skills.includes('bar'), 'A valid skills dir is still scanned despite a broken sibling path');
+
+  await fs.rm(fixtureDir, { recursive: true, force: true });
+});