refactor: export pure functions in build package, fix CLI dispatch, add tests

PROBLEMS FIXED:

1. CLI subcommands no-op (nextdns-skills-build)
   build, validate, search, export, extract-tests, migrate all produced
   no output when dispatched via the unified CLI entry point.
   Root cause: validate.ts and search.ts had isMain guards added; the
   other modules would break if imported. build/cli.ts still used
   side-effect imports which are blocked by isMain checks.

2. CLI subcommands no-op (nextdns-skills-scripts) — same pattern.
   validate-rules, update-counts, check-duplicates, check-tags,
   generate-stats all silently exited 0 with no output.

ROOT CAUSE (both packages):
   cli.ts dispatched via dynamic import() relying on top-level side-effects.
   When a module is dynamically imported:
     import.meta.url  = file:///dist/module.mjs   ← the imported file
     process.argv[1]  = file:///dist/cli.mjs       ← the entry point
   isMain check always fails → nothing runs.

FIX:
   Each module now exports run() containing the CLI execution logic.
   cli.ts (both packages) calls mod.run() explicitly after import.
   isMain guard kept for direct node invocation (node ./module.mjs).

CHANGES:

packages/nextdns-skills-build/src/:
  validate.ts      — export function run() + isMain guard
  search.ts        — extract isMain block into export function run()
  build.ts         — export const run = build; isMain guard
  export.ts        — wrap bare top-level code in export function run()
  extract-tests.ts — export const run = extractTests; isMain guard
  migrate.ts       — export const run = migrate; isMain guard
  cli.ts           — dispatch via mod.run() instead of side-effect import

packages/nextdns-scripts/src/:
  (same pattern already applied in previous commits on origin/develop)

packages/nextdns-skills-build/src/__tests__/:
  validate.test.ts  — NEW: 12 tests for validateRule() (title, explanation,
                      impact, type, file path and ruleId propagation)

packages/nextdns-scripts/src/__tests__/:
  utils.test.ts  — NEW: 20 tests for walkDir(), parseFrontmatter(),
                   collectRuleFiles()

VERIFIED:
  nextdns-skills-build: all 6 subcommands produce correct output
  nextdns-skills-scripts: all 5 subcommands produce correct output
  143/143 tests pass across both packages

Author: tuanductran

packages/nextdns-scripts/src/__tests__/utils.test.ts

@@ -5,195 +5,152 @@ import { afterEach, beforeEach, describe, expect, it } from 'vite-plus/test';
 
 import { collectRuleFiles, parseFrontmatter, walkDir } from '../utils.js';
 
-// ─── Helpers ────────────────────────────────────────────────────────────────
+// ─── Helpers ─────────────────────────────────────────────────────────────────
 
-function tmpDir(): string {
-  return fs.mkdtempSync(path.join(os.tmpdir(), 'ndns-test-'));
-}
+let tmpRoot: string;
 
-function write(dir: string, relPath: string, content = ''): string {
-  const full = path.join(dir, relPath);
+function write(rel: string, content = ''): string {
+  const full = path.join(tmpRoot, rel);
   fs.mkdirSync(path.dirname(full), { recursive: true });
   fs.writeFileSync(full, content, 'utf8');
   return full;
 }
 
-// ─── walkDir ────────────────────────────────────────────────────────────────
+beforeEach(() => {
+  tmpRoot = fs.mkdtempSync(path.join(os.tmpdir(), 'ndns-utils-'));
+});
+
+afterEach(() => {
+  fs.rmSync(tmpRoot, { recursive: true, force: true });
+});
+
+// ─── walkDir ─────────────────────────────────────────────────────────────────
 
 describe('walkDir', () => {
-  let dir: string;
-  beforeEach(() => {
-    dir = tmpDir();
-  });
-  afterEach(() => {
-    fs.rmSync(dir, { recursive: true, force: true });
+  it('returns empty array for non-existent directory', () => {
+    expect(walkDir(path.join(tmpRoot, 'missing'), () => true)).toEqual([]);
   });
 
   it('returns empty array for empty directory', () => {
-    expect(walkDir(dir, () => true)).toEqual([]);
-  });
-
-  it('returns empty array for non-existent directory', () => {
-    expect(walkDir('/does/not/exist', () => true)).toEqual([]);
+    fs.mkdirSync(path.join(tmpRoot, 'empty'));
+    expect(walkDir(path.join(tmpRoot, 'empty'), () => true)).toEqual([]);
   });
 
-  it('collects files matching the predicate', () => {
-    write(dir, 'a.md');
-    write(dir, 'b.ts');
-    write(dir, 'c.md');
-    const results = walkDir(dir, (n) => n.endsWith('.md'));
+  it('collects files matching predicate', () => {
+    write('a/one.md');
+    write('a/two.md');
+    write('a/skip.txt');
+    const results = walkDir(path.join(tmpRoot, 'a'), (n) => n.endsWith('.md'));
     expect(results).toHaveLength(2);
     expect(results.every((f) => f.endsWith('.md'))).toBe(true);
   });
 
   it('recurses into subdirectories', () => {
-    write(dir, 'sub/deep/rule.md');
-    write(dir, 'root.md');
-    const results = walkDir(dir, (n) => n.endsWith('.md'));
-    expect(results).toHaveLength(2);
+    write('root/sub1/a.md');
+    write('root/sub1/sub2/b.md');
+    write('root/c.md');
+    const results = walkDir(path.join(tmpRoot, 'root'), (n) => n.endsWith('.md'));
+    expect(results).toHaveLength(3);
   });
 
-  it('does not return files that do not match predicate', () => {
-    write(dir, 'file.json');
-    const results = walkDir(dir, (n) => n.endsWith('.md'));
-    expect(results).toHaveLength(0);
+  it('excludes files that do not match predicate', () => {
+    write('dir/keep.ts');
+    write('dir/skip.md');
+    const results = walkDir(path.join(tmpRoot, 'dir'), (n) => n.endsWith('.ts'));
+    expect(results).toHaveLength(1);
+    expect(results[0]).toMatch(/keep\.ts$/);
   });
 });
 
-// ─── parseFrontmatter ───────────────────────────────────────────────────────
+// ─── parseFrontmatter ─────────────────────────────────────────────────────────
 
 describe('parseFrontmatter', () => {
   it('returns empty object when no frontmatter', () => {
-    expect(parseFrontmatter('# Hello\nNo frontmatter here.')).toEqual({});
+    expect(parseFrontmatter('# No frontmatter here')).toEqual({});
   });
 
-  it('returns empty object when content does not start with ---', () => {
-    expect(parseFrontmatter('title: foo\n---')).toEqual({});
+  it('returns empty object when frontmatter is not closed', () => {
+    expect(parseFrontmatter('---\ntitle: Test\n')).toEqual({});
   });
 
-  it('parses scalar string fields', () => {
-    const md = `---
-title: 'Authentication'
-impact: HIGH
-type: capability
----
-# Body`;
-    const fm = parseFrontmatter(md);
-    expect(fm['title']).toBe('Authentication');
+  it('parses simple string fields', () => {
+    const fm = parseFrontmatter('---\ntitle: Hello\nimpact: HIGH\n---\n');
+    expect(fm['title']).toBe('Hello');
     expect(fm['impact']).toBe('HIGH');
-    expect(fm['type']).toBe('capability');
-  });
-
-  it('parses YAML array fields', () => {
-    const md = `---
-tags:
-  - api
-  - security
-  - authentication
----`;
-    const fm = parseFrontmatter(md);
-    expect(fm['tags']).toEqual(['api', 'security', 'authentication']);
   });
 
   it('strips surrounding quotes from values', () => {
-    const md = `---
-title: "Rate Limiting"
-impactDescription: 'Some description'
----`;
-    const fm = parseFrontmatter(md);
-    expect(fm['title']).toBe('Rate Limiting');
-    expect(fm['impactDescription']).toBe('Some description');
-  });
-
-  it('handles multiple fields including mixed scalar and array', () => {
-    const md = `---
-title: 'My Rule'
-impact: MEDIUM
-tags:
-  - one
-  - two
-type: efficiency
----`;
-    const fm = parseFrontmatter(md);
-    expect(fm['title']).toBe('My Rule');
-    expect(fm['impact']).toBe('MEDIUM');
-    expect(fm['tags']).toEqual(['one', 'two']);
-    expect(fm['type']).toBe('efficiency');
+    const fm = parseFrontmatter("---\ntitle: 'Quoted Title'\n---\n");
+    expect(fm['title']).toBe('Quoted Title');
   });
 
-  it('returns empty object when closing --- is missing', () => {
-    const md = `---
-title: Missing close
-`;
-    expect(parseFrontmatter(md)).toEqual({});
+  it('parses YAML array into string[]', () => {
+    const fm = parseFrontmatter('---\ntags:\n  - dns\n  - api\n  - setup\n---\n');
+    expect(fm['tags']).toEqual(['dns', 'api', 'setup']);
   });
-});
 
-// ─── collectRuleFiles ────────────────────────────────────────────────────────
-
-describe('collectRuleFiles', () => {
-  let dir: string;
-  beforeEach(() => {
-    dir = tmpDir();
-  });
-  afterEach(() => {
-    fs.rmSync(dir, { recursive: true, force: true });
+  it('returns empty array for empty YAML array', () => {
+    const fm = parseFrontmatter('---\ntags:\n---\n');
+    expect(fm['tags']).toEqual([]);
   });
 
-  it('returns empty array for empty directory', () => {
-    expect(collectRuleFiles(dir)).toEqual([]);
+  it('handles multiple fields and arrays', () => {
+    const content = [
+      '---',
+      "title: 'My Rule'",
+      'impact: MEDIUM',
+      'type: capability',
+      'tags:',
+      '  - cli',
+      '  - dns',
+      '---',
+      '# Body',
+    ].join('\n');
+    const fm = parseFrontmatter(content);
+    expect(fm['title']).toBe('My Rule');
+    expect(fm['impact']).toBe('MEDIUM');
+    expect(fm['tags']).toEqual(['cli', 'dns']);
   });
 
-  it('collects .md files', () => {
-    write(dir, 'rule-one.md');
-    write(dir, 'rule-two.md');
-    expect(collectRuleFiles(dir)).toHaveLength(2);
+  it('handles multiline impactDescription', () => {
+    const content = "---\ntitle: 'Test'\nimpactDescription: 'A long description'\n---\n";
+    const fm = parseFrontmatter(content);
+    expect(fm['impactDescription']).toBe('A long description');
   });
+});
 
-  it('excludes files starting with underscore', () => {
-    write(dir, '_draft.md');
-    write(dir, 'real.md');
-    const results = collectRuleFiles(dir);
-    expect(results).toHaveLength(1);
-    expect(results[0]).toContain('real.md');
-  });
+// ─── collectRuleFiles ─────────────────────────────────────────────────────────
 
-  it('excludes README.md', () => {
-    write(dir, 'README.md');
-    write(dir, 'actual-rule.md');
-    const results = collectRuleFiles(dir);
-    expect(results).toHaveLength(1);
-    expect(results[0]).toContain('actual-rule.md');
+describe('collectRuleFiles', () => {
+  it('excludes SKILL.md', () => {
+    write('rules/SKILL.md');
+    write('rules/auth.md');
+    const results = collectRuleFiles(path.join(tmpRoot, 'rules'));
+    expect(results.every((f) => !f.endsWith('SKILL.md'))).toBe(true);
   });
 
-  it('excludes SKILL.md', () => {
-    write(dir, 'SKILL.md');
-    write(dir, 'rule.md');
-    const results = collectRuleFiles(dir);
-    expect(results).toHaveLength(1);
+  it('excludes README.md', () => {
+    write('rules/README.md');
+    write('rules/setup.md');
+    const results = collectRuleFiles(path.join(tmpRoot, 'rules'));
+    expect(results.every((f) => !f.endsWith('README.md'))).toBe(true);
   });
 
-  it('excludes non-.md files', () => {
-    write(dir, 'script.ts');
-    write(dir, 'config.json');
-    write(dir, 'rule.md');
-    expect(collectRuleFiles(dir)).toHaveLength(1);
+  it('excludes files starting with underscore', () => {
+    write('rules/_draft.md');
+    write('rules/published.md');
+    const results = collectRuleFiles(path.join(tmpRoot, 'rules'));
+    expect(results.every((f) => !path.basename(f).startsWith('_'))).toBe(true);
   });
 
-  it('recurses into subdirectories', () => {
-    write(dir, 'nuxt/api-proxy.md');
-    write(dir, 'nextjs/api-proxy.md');
-    write(dir, 'top-level.md');
-    const results = collectRuleFiles(dir);
-    expect(results).toHaveLength(3);
+  it('includes normal .md files in subdirs', () => {
+    write('rules/nuxt/api-proxy.md');
+    write('rules/nextjs/error-handling.md');
+    const results = collectRuleFiles(path.join(tmpRoot, 'rules'));
+    expect(results).toHaveLength(2);
   });
 
-  it('returns sorted file paths', () => {
-    write(dir, 'zzz.md');
-    write(dir, 'aaa.md');
-    write(dir, 'mmm.md');
-    const results = collectRuleFiles(dir);
-    expect(results[0]).toContain('aaa.md');
-    expect(results[2]).toContain('zzz.md');
+  it('returns empty array for non-existent directory', () => {
+    expect(collectRuleFiles(path.join(tmpRoot, 'missing'))).toEqual([]);
   });
 });

packages/nextdns-scripts/src/check-duplicates.ts

@@ -159,8 +159,7 @@ export function checkDuplicateTags(rules: RuleEntry[]): number {
   return issues;
 }
 
-// Run only when executed directly (not imported as a module in tests)
-if (fileURLToPath(import.meta.url) === process.argv[1]) {
+export function run(): void {
   const rules = loadAllRules();
   console.log(`Loaded ${rules.length} rules\n`);
 
@@ -183,3 +182,6 @@ if (fileURLToPath(import.meta.url) === process.argv[1]) {
   }
   process.exit(0);
 }
+
+// Run only when executed directly (not imported as a module in tests)
+if (fileURLToPath(import.meta.url) === process.argv[1]) run();

packages/nextdns-scripts/src/check-tags.ts

@@ -251,7 +251,9 @@ export function validateTags(skillsDir: string = SKILLS_DIR): boolean {
   return true;
 }
 
-if (fileURLToPath(import.meta.url) === process.argv[1]) {
+export function run(): void {
   const ok = validateTags();
   process.exit(ok ? 0 : 1);
 }
+
+if (fileURLToPath(import.meta.url) === process.argv[1]) run();

packages/nextdns-scripts/src/cli.ts

@@ -2,10 +2,11 @@
 /**
  * Unified CLI entrypoint for nextdns-skills-scripts.
  *
- * Each subcommand module below is a self-invoking script that reads its
- * own arguments from `process.argv.slice(2)`. This wrapper strips the
- * subcommand name off `process.argv` before dynamically importing the
- * target module, so existing module code needs no changes.
+ * Each subcommand module exports a run() function that contains the
+ * logic previously executed as a top-level side-effect. This wrapper
+ * strips the subcommand name off `process.argv` before dynamically
+ * importing the target module and calling run(), so the isMain guard
+ * inside each module is never the dispatch mechanism.
  *
  * Usage:
  *   nextdns-skills-scripts <command> [options]
@@ -45,10 +46,14 @@ async function main(): Promise<void> {
     process.exit(1);
   }
 
-  // Re-slice argv so the target module's own `process.argv.slice(2)` sees `rest`.
+  // Re-slice argv so each module's process.argv.slice(2) sees only `rest`
   process.argv = [process.argv[0] ?? 'node', process.argv[1] ?? 'nextdns-skills-scripts', ...rest];
 
-  await import(modulePath);
+  // Import the module and call its exported run() function explicitly.
+  // We no longer rely on top-level side-effects so that modules remain
+  // importable in tests without executing CLI logic.
+  const mod = (await import(modulePath)) as { run: () => void };
+  mod.run();
 }
 
 void main();

packages/nextdns-scripts/src/generate-stats.ts

@@ -158,7 +158,7 @@ export function printText(report: StatsReport): void {
 
 /* ========= Entry Point ========= */
 
-if (fileURLToPath(import.meta.url) === process.argv[1]) {
+export function run(): void {
   const args = process.argv.slice(2);
   const outputMode: 'json' | 'text' = args.includes('--text') ? 'text' : 'json';
   const report = buildReport();
@@ -168,3 +168,5 @@ if (fileURLToPath(import.meta.url) === process.argv[1]) {
     console.log(JSON.stringify(report, null, 2));
   }
 }
+
+if (fileURLToPath(import.meta.url) === process.argv[1]) run();

packages/nextdns-scripts/src/update-counts.ts

@@ -111,6 +111,8 @@ export function updateCounts(repoRoot: string = REPO_ROOT, skillsDir: string = S
 
 /* ========= Entry Point ========= */
 
-if (fileURLToPath(import.meta.url) === process.argv[1]) {
+export function run(): void {
   updateCounts();
 }
+
+if (fileURLToPath(import.meta.url) === process.argv[1]) run();

packages/nextdns-scripts/src/validate-rules.ts

@@ -201,7 +201,7 @@ export function validateFrontmatter(skillsDir: string = SKILLS_DIR): boolean {
 
 /* ========= Entry Point ========= */
 
-if (fileURLToPath(import.meta.url) === process.argv[1]) {
+export function run(): void {
   const integrityOk = validateReferentialIntegrity();
   const frontmatterOk = validateFrontmatter();
 
@@ -213,3 +213,5 @@ if (fileURLToPath(import.meta.url) === process.argv[1]) {
   console.log(`\n${RED}❌ Validations failed.${NC}`);
   process.exit(1);
 }
+
+if (fileURLToPath(import.meta.url) === process.argv[1]) run();