Skip to content

Commit c6c83ac

Browse files
committed
fix(cli): fail before conflict deletion when skill source missing
1 parent 99828a1 commit c6c83ac

3 files changed

Lines changed: 26 additions & 4 deletions

File tree

.cursor/BUGBOT.md

Lines changed: 4 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,8 @@ export const handler = (p: FooBarParams) => fooBarLogic(p);
4545

4646
## 3. Testing Checklist
4747

48-
* **Ban on Vitest mocking** (`vi.mock`, `vi.fn`, `vi.spyOn`, `.mock*`) ⇒ critical. Use `createMockExecutor` / `createMockFileSystemExecutor`.
48+
* **External-boundary rule**: Use `createMockExecutor` / `createMockFileSystemExecutor` for command execution and filesystem side effects.
49+
* **Internal mocking is allowed**: `vi.mock`, `vi.fn`, `vi.spyOn`, and `.mock*` are acceptable for internal modules/collaborators.
4950
* Each tool must have tests covering happy-path **and** at least one failure path.
5051
* Avoid the `any` type unless justified with an inline comment.
5152

@@ -72,8 +73,8 @@ export const handler = (p: FooBarParams) => fooBarLogic(p);
7273

7374
### How Bugbot Can Verify Rules
7475

75-
1. **Mocking violations**: search `*.test.ts` for `vi.` → critical.
76-
2. **DI compliance**: search for direct `child_process` / `fs` imports outside executors.
76+
1. **External-boundary violations**: confirm tests use injected executors/filesystem for external side effects.
77+
2. **DI compliance**: search for direct `child_process` / `fs` imports outside approved patterns.
7778
3. **Docs accuracy**: compare `docs/TOOLS.md` against `src/mcp/tools/**`.
7879
4. **Style**: ensure ESLint and Prettier pass (`npm run lint`, `npm run format:check`).
7980

src/cli/commands/__tests__/init.test.ts

Lines changed: 21 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -350,6 +350,27 @@ describe('init command', () => {
350350
await expect(app.parseAsync()).rejects.toThrow('Skill source not found');
351351
});
352352

353+
it('does not delete conflicting skill when source file is missing', async () => {
354+
rmSync(join(fakeResourceRoot, 'skills', 'xcodebuildmcp-cli', 'SKILL.md'));
355+
356+
const dest = join(tempDir, 'skills');
357+
const conflictDir = join(dest, 'xcodebuildmcp');
358+
mkdirSync(conflictDir, { recursive: true });
359+
writeFileSync(join(conflictDir, 'SKILL.md'), 'existing mcp skill', 'utf8');
360+
361+
const yargs = (await import('yargs')).default;
362+
const mod = await loadInitModule();
363+
364+
const app = yargs(['init', '--dest', dest, '--skill', 'cli', '--remove-conflict'])
365+
.scriptName('')
366+
.fail(false);
367+
mod.registerInitCommand(app);
368+
369+
await expect(app.parseAsync()).rejects.toThrow('Skill source not found');
370+
expect(existsSync(conflictDir)).toBe(true);
371+
expect(readFileSync(join(conflictDir, 'SKILL.md'), 'utf8')).toBe('existing mcp skill');
372+
});
373+
353374
it('errors when no clients detected and no --dest or --print', async () => {
354375
const emptyHome = join(tempDir, 'empty-home');
355376
mkdirSync(emptyHome, { recursive: true });

src/cli/commands/init.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -98,6 +98,7 @@ async function installSkill(
9898
const targetDir = path.join(skillsDir, skillDirName(skillType));
9999
const altDir = path.join(skillsDir, altSkillDirName(skillType));
100100
const targetFile = path.join(targetDir, 'SKILL.md');
101+
const content = readSkillContent(skillType);
101102

102103
if (fs.existsSync(altDir)) {
103104
if (opts.removeConflict) {
@@ -132,7 +133,6 @@ async function installSkill(
132133
}
133134
}
134135

135-
const content = readSkillContent(skillType);
136136
fs.mkdirSync(targetDir, { recursive: true });
137137
fs.writeFileSync(targetFile, content, 'utf8');
138138

0 commit comments

Comments
 (0)