Skip to content

#629 Skip .DS_Store and stray entries during rovodev install #633

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
williamthorsen merged 1 commit into from
May 19, 2026
+91 −43
Merged

#629 Skip .DS_Store and stray entries during rovodev install #633

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
109 changes: 73 additions & 36 deletions packages/agents/src/commands/__tests__/install.test.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,11 +6,11 @@ import path from 'node:path';
import yaml from 'js-yaml';
import { afterEach, beforeEach, describe, expect, it, vi } from 'vitest';

import { resolveContentDir } from '../../lib/content-resolver.js';
import { readManifest } from '../../lib/manifest.js';
import { getManifestPath } from '../../lib/manifest.js';
import type { InstallOptions } from '../../lib/types.js';
import { installCommand } from '../install.js';
import { resolveContentDir } from '../../lib/content-resolver.ts';
import { readManifest } from '../../lib/manifest.ts';
import { getManifestPath } from '../../lib/manifest.ts';
import type { InstallOptions } from '../../lib/types.ts';
import { installCommand } from '../install.ts';

describe('installCommand', () => {
let tempDir: string;
Expand All @@ -34,7 +34,7 @@ describe('installCommand', () => {
};
}

it('should error when target skills directory is a symlink', async () => {
it('throws when target skills directory is a symlink', async () => {
// Set up the claude home directory with a symlinked skills dir
const claudeHome = path.join(tempDir, '.claude');
const realSkills = path.join(tempDir, 'real-skills');
Expand All @@ -45,7 +45,7 @@ describe('installCommand', () => {
await expect(installCommand(makeOptions(), tempDir)).rejects.toThrow('Target directory is a symlink');
});

it('should install skills and subagents in dry-run mode without writing files', async () => {
it('installs skills and subagents in dry-run mode without writing files', async () => {
const claudeHome = path.join(tempDir, '.claude');
await mkdir(path.join(claudeHome, 'skills'), { recursive: true });
await mkdir(path.join(claudeHome, 'agents'), { recursive: true });
Expand All @@ -61,7 +61,7 @@ describe('installCommand', () => {
expect(skillsContents).toHaveLength(0);
});

it('should install skills and subagents with merged frontmatter', async () => {
it('installs skills and subagents with merged frontmatter', async () => {
const claudeHome = path.join(tempDir, '.claude');
await mkdir(path.join(claudeHome, 'skills'), { recursive: true });
await mkdir(path.join(claudeHome, 'agents'), { recursive: true });
Expand Down Expand Up @@ -94,7 +94,7 @@ describe('installCommand', () => {
expect(claudeManifest?.entries.length).toBeGreaterThan(0);
});

it('should be idempotent - running twice produces the same result', async () => {
it('is idempotent', async () => {
const claudeHome = path.join(tempDir, '.claude');
await mkdir(path.join(claudeHome, 'skills'), { recursive: true });
await mkdir(path.join(claudeHome, 'agents'), { recursive: true });
Expand All @@ -108,7 +108,7 @@ describe('installCommand', () => {
expect(secondContent).toBe(firstContent);
});

it('should skip modified subagent files on re-install without --force', async () => {
it('skips modified subagent files on re-install without --force', async () => {
const claudeHome = path.join(tempDir, '.claude');
await mkdir(path.join(claudeHome, 'skills'), { recursive: true });
await mkdir(path.join(claudeHome, 'agents'), { recursive: true });
Expand All @@ -130,7 +130,7 @@ describe('installCommand', () => {
expect(afterReinstall).toBe(modifiedContent);
});

it('should prefix warn lines with ⚠️ and success summary lines with ✅', async () => {
it('prefixes warn lines with ⚠️ and success summary lines with ✅', async () => {
const claudeHome = path.join(tempDir, '.claude');
await mkdir(path.join(claudeHome, 'skills'), { recursive: true });
await mkdir(path.join(claudeHome, 'agents'), { recursive: true });
Expand Down Expand Up @@ -158,7 +158,7 @@ describe('installCommand', () => {
expect(infoLines.some((line) => line.includes('✅ Installed '))).toBe(true);
});

it('should overwrite modified subagent files on re-install with --force', async () => {
it('overwrites modified subagent files on re-install with --force', async () => {
const claudeHome = path.join(tempDir, '.claude');
await mkdir(path.join(claudeHome, 'skills'), { recursive: true });
await mkdir(path.join(claudeHome, 'agents'), { recursive: true });
Expand All @@ -181,7 +181,7 @@ describe('installCommand', () => {
expect(afterReinstall).toBe(originalContent);
});

it('should copy skills even when link mode is enabled', async () => {
it('copies skills even when link mode is enabled', async () => {
const claudeHome = path.join(tempDir, '.claude');
await mkdir(path.join(claudeHome, 'skills'), { recursive: true });
await mkdir(path.join(claudeHome, 'agents'), { recursive: true });
Expand Down Expand Up @@ -225,7 +225,7 @@ describe('installCommand', () => {
}
});

it('should install claude-specific skills and exclude rovodev-specific skills', async () => {
it('installs claude-specific skills and exclude rovodev-specific skills', async () => {
const claudeHome = path.join(tempDir, '.claude');
await mkdir(path.join(claudeHome, 'skills'), { recursive: true });
await mkdir(path.join(claudeHome, 'agents'), { recursive: true });
Expand All @@ -240,7 +240,7 @@ describe('installCommand', () => {
expect(skillsContents).not.toContain('systematic-debugging');
});

it('should install rovodev-specific skills and exclude claude-specific skills', async () => {
it('installs rovodev-specific skills and excludes claude-specific skills', async () => {
const rovodevHome = path.join(tempDir, '.rovodev');
await mkdir(path.join(rovodevHome, 'skills'), { recursive: true });
await mkdir(path.join(rovodevHome, 'subagents'), { recursive: true });
Expand All @@ -255,7 +255,7 @@ describe('installCommand', () => {
expect(skillsContents).not.toContain('review-permissions');
});

it('should install _data support directory but not _platforms', async () => {
it('installs _data support directory but not _platforms', async () => {
const claudeHome = path.join(tempDir, '.claude');
await mkdir(path.join(claudeHome, 'skills'), { recursive: true });
await mkdir(path.join(claudeHome, 'agents'), { recursive: true });
Expand All @@ -273,7 +273,7 @@ describe('installCommand', () => {
expect(dataContents).toContain('title-templates.md');
});

it('should generate prompts.yml for rovodev with valid YAML structure', async () => {
it('generates prompts.yml for rovodev with valid YAML structure', async () => {
const rovodevHome = path.join(tempDir, '.rovodev');
await mkdir(path.join(rovodevHome, 'skills'), { recursive: true });
await mkdir(path.join(rovodevHome, 'subagents'), { recursive: true });
Expand Down Expand Up @@ -340,7 +340,44 @@ describe('installCommand', () => {
expect(skillNames).toContain('create-pr');
});

it('should track prompts.yml in the manifest for rovodev', async () => {
it('tolerates stray entries (e.g. .DS_Store) in the destination skills directory', async () => {
const rovodevHome = path.join(tempDir, '.rovodev');
const rovodevSkills = path.join(rovodevHome, 'skills');
await mkdir(rovodevSkills, { recursive: true });
await mkdir(path.join(rovodevHome, 'subagents'), { recursive: true });

// Both stray entries exercise the same path: joining `SKILL.md` onto a regular file raises ENOTDIR,
// which the catch swallows. The two shapes (dotfile + plain) cover the common (.DS_Store) and the
// general (any non-directory entry) cases through one mechanism.
await writeFile(path.join(rovodevSkills, '.DS_Store'), '', 'utf8');
await writeFile(path.join(rovodevSkills, 'stray-file'), '', 'utf8');

await expect(installCommand(makeOptions({ platform: 'rovodev' }), tempDir)).resolves.toBeUndefined();

const promptsPath = path.join(rovodevHome, 'prompts.yml');
const content = await readFile(promptsPath, 'utf8');
const parsed: unknown = yaml.load(content);

if (typeof parsed !== 'object' || parsed === null || !('prompts' in parsed)) {
throw new Error('Expected parsed YAML to have a prompts key');
}
if (!Array.isArray(parsed.prompts)) {
throw new TypeError('Expected prompts to be an array');
}
const prompts: Array<unknown> = parsed.prompts;
const skillNames = prompts.map((entry) => {
if (typeof entry !== 'object' || entry === null || !('name' in entry)) {
throw new Error('Expected prompt entry with name');
}
return entry.name;
});

expect(prompts.length).toBeGreaterThan(0);
expect(skillNames).not.toContain('.DS_Store');
expect(skillNames).not.toContain('stray-file');
});

it('tracks prompts.yml in the manifest for rovodev', async () => {
const rovodevHome = path.join(tempDir, '.rovodev');
await mkdir(path.join(rovodevHome, 'skills'), { recursive: true });
await mkdir(path.join(rovodevHome, 'subagents'), { recursive: true });
Expand All @@ -357,7 +394,7 @@ describe('installCommand', () => {
expect(promptsEntry?.contentHash).toMatch(/^sha256:/);
});

it('should NOT generate prompts.yml for claude', async () => {
it('does NOT generate prompts.yml for claude', async () => {
const claudeHome = path.join(tempDir, '.claude');
await mkdir(path.join(claudeHome, 'skills'), { recursive: true });
await mkdir(path.join(claudeHome, 'agents'), { recursive: true });
Expand All @@ -368,7 +405,7 @@ describe('installCommand', () => {
expect(existsSync(promptsPath)).toBe(false);
});

it('should skip modified shared skill on re-install without --force (rovodev)', async () => {
it('skips modified shared skill on re-install without --force (rovodev)', async () => {
const rovodevHome = path.join(tempDir, '.rovodev');
await mkdir(path.join(rovodevHome, 'skills'), { recursive: true });
await mkdir(path.join(rovodevHome, 'subagents'), { recursive: true });
Expand All @@ -391,7 +428,7 @@ describe('installCommand', () => {
expect(afterReinstall).toBe(modifiedContent);
});

it('should overwrite modified subagent on re-install with --force (rovodev)', async () => {
it('overwrites modified subagent on re-install with --force (rovodev)', async () => {
const rovodevHome = path.join(tempDir, '.rovodev');
await mkdir(path.join(rovodevHome, 'skills'), { recursive: true });
await mkdir(path.join(rovodevHome, 'subagents'), { recursive: true });
Expand All @@ -414,7 +451,7 @@ describe('installCommand', () => {
expect(afterReinstall).toBe(originalContent);
});

it('should skip modified prompts.yml on re-install without --force', async () => {
it('skips modified prompts.yml on re-install without --force', async () => {
const rovodevHome = path.join(tempDir, '.rovodev');
await mkdir(path.join(rovodevHome, 'skills'), { recursive: true });
await mkdir(path.join(rovodevHome, 'subagents'), { recursive: true });
Expand All @@ -436,7 +473,7 @@ describe('installCommand', () => {
expect(afterReinstall).toBe(modifiedContent);
});

it('should regenerate modified prompts.yml on re-install with --force', async () => {
it('regenerates modified prompts.yml on re-install with --force', async () => {
const rovodevHome = path.join(tempDir, '.rovodev');
await mkdir(path.join(rovodevHome, 'skills'), { recursive: true });
await mkdir(path.join(rovodevHome, 'subagents'), { recursive: true });
Expand All @@ -459,7 +496,7 @@ describe('installCommand', () => {
expect(afterReinstall).toBe(originalContent);
});

it('should install rovodev in dry-run mode without writing prompts.yml or manifest', async () => {
it('installs rovodev in dry-run mode without writing prompts.yml or manifest', async () => {
const rovodevHome = path.join(tempDir, '.rovodev');
await mkdir(path.join(rovodevHome, 'skills'), { recursive: true });
await mkdir(path.join(rovodevHome, 'subagents'), { recursive: true });
Expand All @@ -479,7 +516,7 @@ describe('installCommand', () => {
expect(existsSync(promptsPath)).toBe(false);
});

it('should copy platform-specific skills even in link mode', async () => {
it('copies platform-specific skills even in link mode', async () => {
const claudeHome = path.join(tempDir, '.claude');
await mkdir(path.join(claudeHome, 'skills'), { recursive: true });
await mkdir(path.join(claudeHome, 'agents'), { recursive: true });
Expand All @@ -500,7 +537,7 @@ describe('installCommand', () => {
expect(reviewPermEntry?.linked).toBe(false);
});

it('should strip surrounding quotes from skill descriptions in prompts.yml', async () => {
it('strips surrounding quotes from skill descriptions in prompts.yml', async () => {
const rovodevHome = path.join(tempDir, '.rovodev');
await mkdir(path.join(rovodevHome, 'skills'), { recursive: true });
await mkdir(path.join(rovodevHome, 'subagents'), { recursive: true });
Expand Down Expand Up @@ -566,7 +603,7 @@ describe('installCommand', () => {
expect(doubleQuotedEntry.description).toBe('A double-quoted description');
});

it('should rewrite relative Markdown link paths to absolute in copy mode', async () => {
it('rewrites relative Markdown link paths to absolute in copy mode', async () => {
const claudeHome = path.join(tempDir, '.claude');
await mkdir(path.join(claudeHome, 'skills'), { recursive: true });
await mkdir(path.join(claudeHome, 'agents'), { recursive: true });
Expand All @@ -584,7 +621,7 @@ describe('installCommand', () => {
expect(content).not.toMatch(/\]\(\.\.\/_data\//);
});

it('should preserve anchor fragments in rewritten paths', async () => {
it('preserves anchor fragments in rewritten paths', async () => {
const claudeHome = path.join(tempDir, '.claude');
await mkdir(path.join(claudeHome, 'skills'), { recursive: true });
await mkdir(path.join(claudeHome, 'agents'), { recursive: true });
Expand All @@ -600,7 +637,7 @@ describe('installCommand', () => {
expect(content).toContain('~/.claude/skills/_data/artifact-conventions.md#finding-scheme-fwtrs--legacy-suffix');
});

it('should rewrite paths in skills even in link mode', async () => {
it('rewrites paths in skills even in link mode', async () => {
const claudeHome = path.join(tempDir, '.claude');
await mkdir(path.join(claudeHome, 'skills'), { recursive: true });
await mkdir(path.join(claudeHome, 'agents'), { recursive: true });
Expand Down Expand Up @@ -764,7 +801,7 @@ describe('installCommand', () => {
});

describe('installScripts', () => {
it('should place script files in the scripts directory after install', async () => {
it('places script files in the scripts directory after install', async () => {
const claudeHome = path.join(tempDir, '.claude');
await mkdir(path.join(claudeHome, 'skills'), { recursive: true });
await mkdir(path.join(claudeHome, 'agents'), { recursive: true });
Expand All @@ -777,7 +814,7 @@ describe('installCommand', () => {
expect(scriptFiles).toContain('describe-change.sh');
});

it('should set executable permissions on copied scripts', async () => {
it('sets executable permissions on copied scripts', async () => {
const claudeHome = path.join(tempDir, '.claude');
await mkdir(path.join(claudeHome, 'skills'), { recursive: true });
await mkdir(path.join(claudeHome, 'agents'), { recursive: true });
Expand All @@ -789,7 +826,7 @@ describe('installCommand', () => {
expect(mode).toBe(0o755);
});

it('should record script entries with sha256 hash and linked: false in copy mode', async () => {
it('records script entries with sha256 hash and linked: false in copy mode', async () => {
const claudeHome = path.join(tempDir, '.claude');
await mkdir(path.join(claudeHome, 'skills'), { recursive: true });
await mkdir(path.join(claudeHome, 'agents'), { recursive: true });
Expand All @@ -807,7 +844,7 @@ describe('installCommand', () => {
}
});

it('should record script entries with linked: true in link mode', async () => {
it('records script entries with linked: true in link mode', async () => {
const claudeHome = path.join(tempDir, '.claude');
await mkdir(path.join(claudeHome, 'skills'), { recursive: true });
await mkdir(path.join(claudeHome, 'agents'), { recursive: true });
Expand All @@ -824,7 +861,7 @@ describe('installCommand', () => {
}
});

it('should skip modified script on re-install without --force', async () => {
it('skips modified script on re-install without --force', async () => {
const claudeHome = path.join(tempDir, '.claude');
await mkdir(path.join(claudeHome, 'skills'), { recursive: true });
await mkdir(path.join(claudeHome, 'agents'), { recursive: true });
Expand All @@ -845,7 +882,7 @@ describe('installCommand', () => {
expect(afterReinstall).toBe(modifiedContent);
});

it('should overwrite modified script on re-install with --force', async () => {
it('overwrites modified script on re-install with --force', async () => {
const claudeHome = path.join(tempDir, '.claude');
await mkdir(path.join(claudeHome, 'skills'), { recursive: true });
await mkdir(path.join(claudeHome, 'agents'), { recursive: true });
Expand All @@ -867,7 +904,7 @@ describe('installCommand', () => {
expect(afterReinstall).toBe(originalContent);
});

it('should not create scripts directory in dry-run mode', async () => {
it('does not create scripts directory in dry-run mode', async () => {
const claudeHome = path.join(tempDir, '.claude');
await mkdir(path.join(claudeHome, 'skills'), { recursive: true });
await mkdir(path.join(claudeHome, 'agents'), { recursive: true });
Expand Down
25 changes: 18 additions & 7 deletions packages/agents/src/commands/install.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
import { chmod, mkdir, readdir, readFile, stat, writeFile } from 'node:fs/promises';
import path from 'node:path';

import { resolveContentDir } from '../lib/content-resolver.js';
import { expandIncludes } from '../lib/directive-expander.js';
import { mergeFrontmatter, parseFrontmatter } from '../lib/frontmatter-merger.js';
import { checkSymlinkSafety, copyItem, linkItem, unlinkIfSymlink } from '../lib/installer.js';
import { resolveContentDir } from '../lib/content-resolver.ts';
import { expandIncludes } from '../lib/directive-expander.ts';
import { mergeFrontmatter, parseFrontmatter } from '../lib/frontmatter-merger.ts';
import { checkSymlinkSafety, copyItem, linkItem, unlinkIfSymlink } from '../lib/installer.ts';
import {
computeContentHash,
detectDrift,
Expand Down Expand Up @@ -518,10 +518,13 @@ async function generatePromptsYml(
try {
skillContent = await readFile(skillMdPath, 'utf8');
} catch (error: unknown) {
if (!isEnoent(error)) {
throw error;
// Tolerate any non-directory entry in the destination skills directory (e.g. a `.DS_Store` left by Finder):
// joining `SKILL.md` onto a regular file raises `ENOTDIR`; a directory without `SKILL.md` raises `ENOENT`.
// Either way the entry is not a skill and should be skipped.
if (isMissingSkill(error)) {
continue;
}
continue;
throw error;
}

const { lines } = parseFrontmatter(skillContent);
Expand Down Expand Up @@ -862,6 +865,14 @@ function isEnoent(error: unknown): boolean {
return typeof error === 'object' && error !== null && 'code' in error && error.code === 'ENOENT';
}

/** True when a `readFile` of `SKILL.md` raised `ENOENT` (file absent) or `ENOTDIR` (parent segment is a regular file). */
function isMissingSkill(error: unknown): boolean {
if (typeof error !== 'object' || error === null || !('code' in error)) {
return false;
}
return error.code === 'ENOENT' || error.code === 'ENOTDIR';
}

/**
* Returns a POSIX-style path label for a skill source file relative to `contentDir`, used as the `contextLabel`
* argument to `rewriteToolNames` so install errors include a stable, platform-independent file reference.
Expand Down
Loading