refactor(skills): SRP — extract findSkillFile + getGlobalSkillsDirectory

Per sweetman PR review (comments r3283710486 and r3283762023). Each
helper now lives in its own file with its own focused test suite:
- lib/skills/findSkillFile.ts — was inlined in discoverSkills.ts
- 3 new unit tests (prefer SKILL.md, fall back to skill.md, null
when neither exists)
- lib/skills/getGlobalSkillsDirectory.ts — was inlined in
getSandboxSkillDirectories.ts
- 2 new unit tests (standard path, trailing-slash tolerance)
discoverSkills now imports findSkillFile. getSandboxSkillDirectories
imports getGlobalSkillsDirectory. The old getSandboxSkillDirectories
test loses its inline getGlobalSkillsDirectory cases (those moved to
the dedicated test file).
Full suite passes; lint clean; production build succeeds.
Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>

Author: sweetmantech

lib/skills/__tests__/findSkillFile.test.ts

@@ -0,0 +1,34 @@
+import{describe,it,expect,vi,beforeEach}from"vitest";
+import{findSkillFile}from"@/lib/skills/findSkillFile";
+
+beforeEach(()=>vi.clearAllMocks());
+
+functionmakeSandbox(existing: string[]){
+constset=newSet(existing);
+return{
+access: vi.fn(async(p: string)=>{
+if(!set.has(p))thrownewError(`ENOENT: ${p}`);
+}),
+};
+}
+
+describe("findSkillFile",()=>{
+it("prefers uppercase SKILL.md when both casings exist",async()=>{
+constsb=makeSandbox(["/skills/foo/SKILL.md","/skills/foo/skill.md"]);
+constresult=awaitfindSkillFile(sbasnever,"/skills/foo");
+expect(result).toBe("/skills/foo/SKILL.md");
+expect(sb.access).toHaveBeenCalledWith("/skills/foo/SKILL.md");
+});
+
+it("falls back to lowercase skill.md when SKILL.md is missing",async()=>{
+constsb=makeSandbox(["/skills/foo/skill.md"]);
+constresult=awaitfindSkillFile(sbasnever,"/skills/foo");
+expect(result).toBe("/skills/foo/skill.md");
+});
+
+it("returns null when neither casing exists",async()=>{
+constsb=makeSandbox([]);
+constresult=awaitfindSkillFile(sbasnever,"/skills/foo");
+expect(result).toBeNull();
+});
+});

lib/skills/__tests__/getGlobalSkillsDirectory.test.ts

@@ -0,0 +1,15 @@
+import{describe,it,expect}from"vitest";
+import{getGlobalSkillsDirectory}from"@/lib/skills/getGlobalSkillsDirectory";
+
+describe("getGlobalSkillsDirectory",()=>{
+it("returns <home>/.agents/skills",()=>{
+expect(getGlobalSkillsDirectory("/root")).toBe("/root/.agents/skills");
+expect(getGlobalSkillsDirectory("/home/vercel-sandbox")).toBe(
+"/home/vercel-sandbox/.agents/skills",
+);
+});
+
+it("handles trailing slash on input",()=>{
+expect(getGlobalSkillsDirectory("/root/")).toBe("/root/.agents/skills");
+});
+});

lib/skills/__tests__/getSandboxSkillDirectories.test.ts

@@ -1,8 +1,5 @@
 import{describe,it,expect,vi,beforeEach}from"vitest";
-import{
-getGlobalSkillsDirectory,
-getSandboxSkillDirectories,
-}from"@/lib/skills/getSandboxSkillDirectories";
+import{getSandboxSkillDirectories}from"@/lib/skills/getSandboxSkillDirectories";
 import{resolveSandboxHomeDirectory}from"@/lib/sandbox/resolveSandboxHomeDirectory";
 
 vi.mock("@/lib/sandbox/resolveSandboxHomeDirectory",()=>({
@@ -11,15 +8,6 @@ vi.mock("@/lib/sandbox/resolveSandboxHomeDirectory", () => ({
 
 beforeEach(()=>vi.clearAllMocks());
 
-describe("getGlobalSkillsDirectory",()=>{
-it("returns <home>/.agents/skills",()=>{
-expect(getGlobalSkillsDirectory("/root")).toBe("/root/.agents/skills");
-expect(getGlobalSkillsDirectory("/home/vercel-sandbox")).toBe(
-"/home/vercel-sandbox/.agents/skills",
-);
-});
-});
-
 describe("getSandboxSkillDirectories",()=>{
 it("returns just the global skill dir under the resolved $HOME",async()=>{
 vi.mocked(resolveSandboxHomeDirectory).mockResolvedValue("/home/vercel-sandbox");

lib/skills/discoverSkills.ts

@@ -1,5 +1,6 @@
 import*aspathfrom"path";
 importtype{Sandbox}from"@/lib/sandbox/interface";
+import{findSkillFile}from"@/lib/skills/findSkillFile";
 import{parseSkillFrontmatter}from"@/lib/skills/parseSkillFrontmatter";
 import{frontmatterToOptions,typeSkillMetadata}from"@/lib/skills/skillTypes";
 
@@ -9,29 +10,6 @@ import { frontmatterToOptions, type SkillMetadata } from "@/lib/skills/skillType
  */
 constBUILTIN_COMMANDS=["model","resume","new"];
 
-/**
- * Locate the SKILL.md file inside a candidate skill directory. Prefers
- * uppercase `SKILL.md`, falls back to lowercase `skill.md`, returns null
- * when neither exists.
- */
-asyncfunctionfindSkillFile(sandbox: Sandbox,skillDir: string): Promise<string|null>{
-constuppercase=path.join(skillDir,"SKILL.md");
-constlowercase=path.join(skillDir,"skill.md");
-
-try{
-awaitsandbox.access(uppercase);
-returnuppercase;
-}catch{
-// try lowercase
-}
-try{
-awaitsandbox.access(lowercase);
-returnlowercase;
-}catch{
-returnnull;
-}
-}
-
 /**
  * Scan a list of directories for skills. Each directory is expected to
  * contain one subdirectory per skill, with a SKILL.md (or skill.md)

lib/skills/findSkillFile.ts

@@ -0,0 +1,33 @@
+import*aspathfrom"path";
+importtype{Sandbox}from"@/lib/sandbox/interface";
+
+/**
+ * Locate the SKILL.md file inside a candidate skill directory. Prefers
+ * uppercase `SKILL.md` (the project convention) but falls back to
+ * lowercase `skill.md` for skills that ship the lowercase name. Returns
+ * `null` when neither file exists so callers can skip the entry.
+ *
+ * Probes via `sandbox.access` (which throws on missing) rather than
+ * `readdir` so we don't pay the cost of listing a directory whose
+ * contents we don't otherwise need.
+ *
+ * @param sandbox - Connected sandbox handle.
+ * @param skillDir - Absolute path to the candidate skill directory.
+ */
+exportasyncfunctionfindSkillFile(sandbox: Sandbox,skillDir: string): Promise<string|null>{
+constuppercase=path.join(skillDir,"SKILL.md");
+constlowercase=path.join(skillDir,"skill.md");
+
+try{
+awaitsandbox.access(uppercase);
+returnuppercase;
+}catch{
+// try lowercase
+}
+try{
+awaitsandbox.access(lowercase);
+returnlowercase;
+}catch{
+returnnull;
+}
+}

lib/skills/getGlobalSkillsDirectory.ts

@@ -0,0 +1,14 @@
+import*aspathfrom"path";
+
+/**
+ * Resolve the absolute path to the global skills directory under a
+ * given `$HOME`. This is where `installSessionGlobalSkills` lays down
+ * skills at sandbox provisioning time via `npx skills add ... -g`
+ * (today: `recoup-api`, `artist-workspace`).
+ *
+ * @param homeDirectory - The sandbox's resolved $HOME (e.g.
+ * `/home/vercel-sandbox`, or `/root` on the open-agents base image).
+ */
+exportfunctiongetGlobalSkillsDirectory(homeDirectory: string): string{
+returnpath.posix.join(homeDirectory,".agents","skills");
+}

lib/skills/getSandboxSkillDirectories.ts

@@ -1,23 +1,12 @@
-import*aspathfrom"path";
 importtype{Sandbox}from"@/lib/sandbox/interface";
 import{resolveSandboxHomeDirectory}from"@/lib/sandbox/resolveSandboxHomeDirectory";
-
-/**
- * Global skill directory under `$HOME`. This is where
- * `installSessionGlobalSkills` lays down skills at sandbox provisioning
- * time via `npx skills add ... -g`. Per project convention all skills
- * are provisioned globally — org repos do not bundle their own skill
- * directories, so there's nothing project-level to scan.
- */
-exportfunctiongetGlobalSkillsDirectory(homeDirectory: string): string{
-returnpath.posix.join(homeDirectory,".agents","skills");
-}
+import{getGlobalSkillsDirectory}from"@/lib/skills/getGlobalSkillsDirectory";
 
 /**
  * Resolve the directory list to scan when discovering skills for a
  * sandbox. Currently just one path — `${HOME}/.agents/skills/` —
- * because skills are always provisioned globally at sandbox startup
- * rather than bundled into the cloned repo.
+ * because all skills are provisioned globally at sandbox startup via
+ * `installSessionGlobalSkills` rather than bundled into the cloned repo.
  *
  * @param sandbox - Connected sandbox handle.
  */