fix(cli): dynamically discover and install agent skills

[cnighut]

Description

This PR fixes #269 by dynamically discovering the available skills in the skills/ directory instead of using a hardcoded SKILL_NAMES array.

Previously, if a new skill was added (such as gitnexus-pr-review), it wouldn't be installed automatically during gitnexus setup unless it was also manually added to the hardcoded array.

Now, installSkillsTo scans the directory and automatically picks up both flat file (.md) and directory-based skills.

Thank you @abhigyanpatwari for creating and maintaining this awesome project!

[vercel]

Someone is attempting to deploy a commit to the NexusCore Team on Vercel.

A member of the Team first needs to authorize it.

[magyargergo]

Hey @cnighut ,

Could you please merge origin/master into this branch? 🙏

Thanks!

[cnighut]

Hey @cnighut ,

Could you please merge origin/master into this branch? 🙏

Thanks!

Done @magyargergo!

[magyargergo]

@claude review this code and recommend a more efficient way of traversing and finding md files like glob for example.

[claude]

Claude finished @magyargergo's task in 1m 34s —— View job

---

Code Review: Glob-based skill discovery

• Read and analyze gitnexus/src/cli/setup.ts
• Review current traversal approach
• Recommend glob-based alternative

---

Current implementation analysis

The installSkillsTo function (lines 252–301) has a correctness issue and an efficiency problem worth addressing:

Redundant fs.stat call (lines 278–282)

readdir is already called with { withFileTypes: true } at line 258, so each entry already knows whether it's a file or directory. However, that type information is discarded — only the name is pushed to skillNames. The inner loop then performs a second fs.stat on every entry to re-determine the same information:

// Line 258 — already has type info ✓
const entries = await fs.readdir(skillsRoot, { withFileTypes: true });

// ... type info is discarded, only name is stored ...

// Line 280 — redundant re-stat for info we already had ✗
const stat = await fs.stat(dirSource);
isDirectory = stat.isDirectory();

This means every skill incurs an extra filesystem round-trip. For N skills = 2N stat-equivalent calls instead of N.

---

Recommended approach: use glob (already a dependency)

glob v11 is already in dependencies — no new package needed. It supports pattern matching and can cleanly separate flat .md files from directory-based skills in one pass:

import { glob } from 'glob';

async function installSkillsTo(targetDir: string): Promise<string[]> {
  const installed: string[] = [];
  const skillsRoot = path.join(__dirname, '..', '..', 'skills');

  // One glob pass per layout — no extra fs.stat needed
  let flatFiles: string[] = [];
  let dirSkills: string[] = [];
  try {
    [flatFiles, dirSkills] = await Promise.all([
      glob('*.md',        { cwd: skillsRoot }),   // flat: skills/foo.md
      glob('*/SKILL.md',  { cwd: skillsRoot }),   // dir:  skills/foo/SKILL.md
    ]);
  } catch {
    return [];
  }

  const skills: Array<{ name: string; isDir: boolean }> = [
    ...flatFiles.map(f  => ({ name: path.basename(f, '.md'), isDir: false })),
    ...dirSkills.map(fp => ({ name: path.dirname(fp),        isDir: true  })),
  ];

  for (const { name, isDir } of skills) {
    const skillDir = path.join(targetDir, name);
    try {
      if (isDir) {
        await copyDirRecursive(path.join(skillsRoot, name), skillDir);
      } else {
        const content = await fs.readFile(path.join(skillsRoot, `${name}.md`), 'utf-8');
        await fs.mkdir(skillDir, { recursive: true });
        await fs.writeFile(path.join(skillDir, 'SKILL.md'), content, 'utf-8');
      }
      installed.push(name);
    } catch { /* skip */ }
  }

  return installed;
}

Benefits over current code:

	Current	Glob-based
stat calls per skill	2 (readdir + re-stat)	0 (type comes from glob match)
Parallel discovery	No	Yes (Promise.all)
Pattern flexibility	Manual filter	Declarative glob patterns
Lines of code	~45	~25

The Promise.all for the two glob calls also parallelises discovery of both layouts, which is a nice bonus.

Fix this →

[github-actions]

CI Report

✅ All checks passed

Pipeline Status

Stage	Status	Details
✅ Typecheck	success	tsc --noEmit
✅ Unit Tests	success	3 platforms
✅ Integration	success	3 OS x 4 groups = 12 jobs

Test Results

✅ 1581 passed
· 507 suites · 1636 total
· ⏱️ 33s
· 📊 1146 unit + 490 integration

Code Coverage

Combined (Unit + Integration)

Metric	Coverage	Covered	Threshold	Status
Statements	48.1%	3713/7719	26%	🟢 █████████░░░░░░░░░░░
Branches	39.69%	2119/5338	23%	🟢 ███████░░░░░░░░░░░░░
Functions	50.67%	414/817	28%	🟢 ██████████░░░░░░░░░░
Lines	49.53%	3441/6946	27%	🟢 █████████░░░░░░░░░░░

Coverage breakdown by test suite

Unit Tests

Metric	Coverage	Covered	Threshold	Status
Statements	36.87%	2846/7719	26%	🟢 ███████░░░░░░░░░░░░░
Branches	31.06%	1658/5338	23%	🟢 ██████░░░░░░░░░░░░░░
Functions	38.8%	317/817	28%	🟢 ███████░░░░░░░░░░░░░
Lines	38.25%	2657/6946	27%	🟢 ███████░░░░░░░░░░░░░

Integration Tests

Metric	Coverage	Covered	Threshold	Status
Statements	24.67%	1905/7719	26%	🔴 ████░░░░░░░░░░░░░░░░
Branches	19.29%	1030/5338	23%	🔴 ███░░░░░░░░░░░░░░░░░
Functions	26.8%	219/817	28%	🔴 █████░░░░░░░░░░░░░░░
Lines	25.35%	1761/6946	27%	🔴 █████░░░░░░░░░░░░░░░

Coverage thresholds are auto-ratcheted — they only go up

Vitest thresholds.autoUpdate bumps the floor whenever local coverage exceeds it.
CI enforces the current thresholds; developers commit the ratcheted values.

---

_{📋 View full run · Generated by CI}

[cnighut]

Good idea. I have switched skill discovery to glob patterns and removed the redundant stat checks in setup flow.

[magyargergo]

@claude can you please recommend them the most valuable integration test for this?

[claude]

Claude finished @magyargergo's task in 1m 22s —— View job

---

Most valuable integration test for glob-based skill discovery

• Read and analyze gitnexus/src/cli/setup.ts
• Review existing integration tests (skills-e2e.test.ts, cli-e2e.test.ts)
• Inspect real skills directory layout
• Formulate recommendation

---

Context

The PR fixes a real bug: the old hardcoded SKILL_NAMES array silently missed new skills (e.g. gitnexus-pr-review.md was present on disk but never installed). The new installSkillsTo uses two parallel glob calls to discover skills dynamically.

The current test suite has no test for the setup/install flow at all — skills-e2e.test.ts only tests gitnexus analyze --skills, not installSkillsTo. That gap is exactly where the bug lived undetected.

---

Recommended test: "newly-added skill is auto-discovered and installed"

This is the single test that would have caught the original bug and validates all three distinct behaviors of the new implementation in one scenario:

// test/integration/skill-install.test.ts
import { describe, it, expect, beforeEach, afterEach } from 'vitest';
import fs from 'fs/promises';
import path from 'path';
import os from 'os';

// ── test helper: call installSkillsTo via a thin re-export ──────────────────
// Add to setup.ts: export { installSkillsTo } (or test via the CLI setup command)
import { installSkillsTo } from '../../src/cli/setup.js';

describe('installSkillsTo – glob discovery', () => {
  let skillsRoot: string;
  let targetDir: string;

  beforeEach(async () => {
    // temp source that mimics the real skills/ directory
    skillsRoot = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-skills-src-'));
    targetDir  = await fs.mkdtemp(path.join(os.tmpdir(), 'gn-skills-dst-'));
  });

  afterEach(async () => {
    await fs.rm(skillsRoot, { recursive: true, force: true });
    await fs.rm(targetDir,  { recursive: true, force: true });
  });

  it('installs a flat .md skill (legacy layout)', async () => {
    await fs.writeFile(path.join(skillsRoot, 'gitnexus-guide.md'), '# Guide', 'utf-8');

    const installed = await installSkillsTo(targetDir, skillsRoot);

    expect(installed).toContain('gitnexus-guide');
    const content = await fs.readFile(
      path.join(targetDir, 'gitnexus-guide', 'SKILL.md'), 'utf-8'
    );
    expect(content).toBe('# Guide');
  });

  it('installs a directory-based skill recursively', async () => {
    const srcDir = path.join(skillsRoot, 'gitnexus-pr-review');
    await fs.mkdir(path.join(srcDir, 'references'), { recursive: true });
    await fs.writeFile(path.join(srcDir, 'SKILL.md'), '# PR Review', 'utf-8');
    await fs.writeFile(path.join(srcDir, 'references', 'extra.md'), '# Extra', 'utf-8');

    const installed = await installSkillsTo(targetDir, skillsRoot);

    expect(installed).toContain('gitnexus-pr-review');
    expect(await fs.readFile(
      path.join(targetDir, 'gitnexus-pr-review', 'SKILL.md'), 'utf-8'
    )).toBe('# PR Review');
    // sub-directory must be copied too
    expect(await fs.readFile(
      path.join(targetDir, 'gitnexus-pr-review', 'references', 'extra.md'), 'utf-8'
    )).toBe('# Extra');
  });

  it('auto-discovers a newly added skill without code changes', async () => {
    // seed known skills
    await fs.writeFile(path.join(skillsRoot, 'gitnexus-guide.md'), '# Guide', 'utf-8');
    // ← this is the skill that was silently missed before the fix
    await fs.writeFile(path.join(skillsRoot, 'gitnexus-pr-review.md'), '# PR Review', 'utf-8');

    const installed = await installSkillsTo(targetDir, skillsRoot);

    // both must be installed; no manual array update required
    expect(installed).toContain('gitnexus-guide');
    expect(installed).toContain('gitnexus-pr-review');
    expect(installed).toHaveLength(2);
  });

  it('directory skill takes precedence over flat .md with the same name', async () => {
    // flat file
    await fs.writeFile(path.join(skillsRoot, 'my-skill.md'), '# Flat', 'utf-8');
    // directory version (richer layout)
    const srcDir = path.join(skillsRoot, 'my-skill');
    await fs.mkdir(srcDir, { recursive: true });
    await fs.writeFile(path.join(srcDir, 'SKILL.md'), '# Dir', 'utf-8');

    const installed = await installSkillsTo(targetDir, skillsRoot);

    // directory wins; only one copy installed
    expect(installed.filter(n => n === 'my-skill')).toHaveLength(1);
    expect(await fs.readFile(
      path.join(targetDir, 'my-skill', 'SKILL.md'), 'utf-8'
    )).toBe('# Dir');
  });
});

---

One required code change to enable this

installSkillsTo currently hard-codes skillsRoot as path.join(__dirname, '..', '..', 'skills'). To make it testable, add a second optional parameter:

// setup.ts  (line 253)
async function installSkillsTo(targetDir: string, skillsRoot?: string): Promise<string[]> {
  const root = skillsRoot ?? path.join(__dirname, '..', '..', 'skills');
  // replace all `skillsRoot` references with `root`

This is a one-line change that doesn't alter production behaviour at all.

---

Why this test is the most valuable

Property	Value
Regression coverage	Directly catches the original bug — a new .md skill silently not installed
Both layouts	Validates flat file AND recursive directory copying
Dedup logic	Exercises the Map-based precedence (directory wins over flat)
No filesystem mocking	Uses real tmp dirs → tests the actual glob + fs path end-to-end
Fast	No CLI spawn, no git repo — runs in ~10 ms

Fix this →

[cnighut]

Added basic integration coverage for setup skill installation to cursor root.
Makes sense to have an integration test.