add confirmation if removing ALL rules profiles, and add --force flag on rules remove
This commit is contained in:
@@ -259,6 +259,9 @@ task-master rules add <rules1,rules2,...>
|
||||
# Remove rule sets from your project
|
||||
task-master rules remove <rules1,rules2,...>
|
||||
|
||||
# Remove rule sets bypassing safety check (dangerous)
|
||||
task-master rules remove <rules1,rules2,...> --force
|
||||
|
||||
# Launch interactive rules setup to select rules
|
||||
# (does not re-initialize project or ask about shell aliases)
|
||||
task-master rules setup
|
||||
@@ -266,6 +269,7 @@ task-master rules setup
|
||||
|
||||
- Adding rules creates the rules directory (e.g., `.roo/rules`) and copies/initializes the rules.
|
||||
- Removing rules deletes the rules directory and associated MCP config.
|
||||
- **Safety Check**: Attempting to remove ALL rules profiles will trigger a critical warning requiring confirmation. Use `--force` to bypass.
|
||||
- You can use multiple comma-separated rules in a single command.
|
||||
- The `setup` action launches an interactive prompt to select which rules to apply. The list of rules is always current with the available profiles, and no manual updates are needed. This command does **not** re-initialize your project or affect shell aliases; it only manages rules interactively.
|
||||
|
||||
|
||||
@@ -15,6 +15,10 @@ import {
|
||||
} from '../../../../src/utils/rule-transformer.js';
|
||||
import { RULES_PROFILES } from '../../../../src/constants/profiles.js';
|
||||
import { RULES_ACTIONS } from '../../../../src/constants/rules-actions.js';
|
||||
import {
|
||||
wouldRemovalLeaveNoProfiles,
|
||||
getInstalledRulesProfiles
|
||||
} from '../../../../src/utils/rules-detection.js';
|
||||
import path from 'path';
|
||||
import fs from 'fs';
|
||||
|
||||
@@ -32,7 +36,7 @@ import fs from 'fs';
|
||||
export async function rulesDirect(args, log, context = {}) {
|
||||
enableSilentMode();
|
||||
try {
|
||||
const { action, profiles, projectRoot, yes } = args;
|
||||
const { action, profiles, projectRoot, yes, force } = args;
|
||||
if (
|
||||
!action ||
|
||||
!Array.isArray(profiles) ||
|
||||
@@ -52,6 +56,21 @@ export async function rulesDirect(args, log, context = {}) {
|
||||
const addResults = [];
|
||||
|
||||
if (action === RULES_ACTIONS.REMOVE) {
|
||||
// Safety check: Ensure this won't remove all rules profiles (unless forced)
|
||||
if (!force && wouldRemovalLeaveNoProfiles(projectRoot, profiles)) {
|
||||
const installedProfiles = getInstalledRulesProfiles(projectRoot);
|
||||
const remainingProfiles = installedProfiles.filter(
|
||||
(profile) => !profiles.includes(profile)
|
||||
);
|
||||
return {
|
||||
success: false,
|
||||
error: {
|
||||
code: 'CRITICAL_REMOVAL_BLOCKED',
|
||||
message: `CRITICAL: This operation would remove ALL remaining rules profiles (${profiles.join(', ')}), leaving your project with no rules configurations. This could significantly impact functionality. Currently installed profiles: ${installedProfiles.join(', ')}. If you're certain you want to proceed, set force: true or use the CLI with --force flag.`
|
||||
}
|
||||
};
|
||||
}
|
||||
|
||||
for (const profile of profiles) {
|
||||
if (!isValidProfile(profile)) {
|
||||
removalResults.push({
|
||||
|
||||
@@ -34,6 +34,13 @@ export function registerRulesTool(server) {
|
||||
.string()
|
||||
.describe(
|
||||
'The root directory of the project. Must be an absolute path.'
|
||||
),
|
||||
force: z
|
||||
.boolean()
|
||||
.optional()
|
||||
.default(false)
|
||||
.describe(
|
||||
'DANGEROUS: Force removal even if it would leave no rules profiles. Only use if you are absolutely certain.'
|
||||
)
|
||||
}),
|
||||
execute: withNormalizedProjectRoot(async (args, { log, session }) => {
|
||||
|
||||
@@ -68,7 +68,14 @@ import {
|
||||
displayApiKeyStatus,
|
||||
displayAiUsageSummary
|
||||
} from './ui.js';
|
||||
import { confirmProfilesRemove } from '../../src/ui/confirm.js';
|
||||
import {
|
||||
confirmProfilesRemove,
|
||||
confirmRemoveAllRemainingProfiles
|
||||
} from '../../src/ui/confirm.js';
|
||||
import {
|
||||
wouldRemovalLeaveNoProfiles,
|
||||
getInstalledRulesProfiles
|
||||
} from '../../src/utils/rules-detection.js';
|
||||
|
||||
import { initializeProject } from '../init.js';
|
||||
import {
|
||||
@@ -2730,8 +2737,16 @@ Examples:
|
||||
if (action === RULES_ACTIONS.REMOVE) {
|
||||
let confirmed = true;
|
||||
if (!options.force) {
|
||||
const ui = await import('./ui.js');
|
||||
confirmed = await confirmProfilesRemove(expandedProfiles);
|
||||
// Check if this removal would leave no profiles remaining
|
||||
if (wouldRemovalLeaveNoProfiles(projectDir, expandedProfiles)) {
|
||||
const installedProfiles = getInstalledRulesProfiles(projectDir);
|
||||
confirmed = await confirmRemoveAllRemainingProfiles(
|
||||
expandedProfiles,
|
||||
installedProfiles
|
||||
);
|
||||
} else {
|
||||
confirmed = await confirmProfilesRemove(expandedProfiles);
|
||||
}
|
||||
}
|
||||
if (!confirmed) {
|
||||
console.log(chalk.yellow('Aborted: No rules were removed.'));
|
||||
|
||||
@@ -31,4 +31,49 @@ This will remove the entire .[profile] directory for each selected profile.\n\nA
|
||||
return confirm;
|
||||
}
|
||||
|
||||
export { confirmProfilesRemove };
|
||||
/**
|
||||
* Confirm removing ALL remaining profile rules (extremely critical operation)
|
||||
* @param {string[]} profiles - Array of profile names to remove
|
||||
* @param {string[]} remainingProfiles - Array of profiles that would be left after removal
|
||||
* @returns {Promise<boolean>} - Promise resolving to true if user confirms, false otherwise
|
||||
*/
|
||||
async function confirmRemoveAllRemainingProfiles(profiles, remainingProfiles) {
|
||||
const profileList = profiles
|
||||
.map((p) => p.charAt(0).toUpperCase() + p.slice(1))
|
||||
.join(', ');
|
||||
|
||||
console.log(
|
||||
boxen(
|
||||
chalk.red.bold(
|
||||
`⚠️ CRITICAL WARNING: REMOVING ALL RULES PROFILES ⚠️\n\n` +
|
||||
`You are about to remove: ${profileList}\n` +
|
||||
`This will leave your project with NO rules profiles remaining!\n\n` +
|
||||
`This could significantly impact functionality and development experience:\n` +
|
||||
`• Loss of IDE-specific rules and conventions\n` +
|
||||
`• No MCP configurations for AI assistants\n` +
|
||||
`• Reduced development guidance and best practices\n\n` +
|
||||
`Are you absolutely sure you want to proceed?`
|
||||
),
|
||||
{
|
||||
padding: 1,
|
||||
borderColor: 'red',
|
||||
borderStyle: 'double',
|
||||
title: '🚨 CRITICAL OPERATION',
|
||||
titleAlignment: 'center'
|
||||
}
|
||||
)
|
||||
);
|
||||
|
||||
const inquirer = await import('inquirer');
|
||||
const { confirm } = await inquirer.default.prompt([
|
||||
{
|
||||
type: 'confirm',
|
||||
name: 'confirm',
|
||||
message: 'Type y to confirm removing ALL rules profiles, or n to abort:',
|
||||
default: false
|
||||
}
|
||||
]);
|
||||
return confirm;
|
||||
}
|
||||
|
||||
export { confirmProfilesRemove, confirmRemoveAllRemainingProfiles };
|
||||
|
||||
48
src/utils/rules-detection.js
Normal file
48
src/utils/rules-detection.js
Normal file
@@ -0,0 +1,48 @@
|
||||
/**
|
||||
* Rules Detection Utility
|
||||
* Helper functions to detect existing rules profiles in a project
|
||||
*/
|
||||
import fs from 'fs';
|
||||
import path from 'path';
|
||||
import { RULES_PROFILES } from '../constants/profiles.js';
|
||||
import { getRulesProfile } from './rule-transformer.js';
|
||||
|
||||
/**
|
||||
* Detect which rules profiles are currently installed in the project
|
||||
* @param {string} projectRoot - Project root directory
|
||||
* @returns {string[]} Array of installed profile names
|
||||
*/
|
||||
export function getInstalledRulesProfiles(projectRoot) {
|
||||
const installedProfiles = [];
|
||||
|
||||
for (const profileName of RULES_PROFILES) {
|
||||
const profileConfig = getRulesProfile(profileName);
|
||||
if (!profileConfig) continue;
|
||||
|
||||
// Check if the profile directory exists
|
||||
const profileDir = path.join(projectRoot, profileConfig.profileDir);
|
||||
const rulesDir = path.join(projectRoot, profileConfig.rulesDir);
|
||||
|
||||
// A profile is considered installed if either the profile dir or rules dir exists
|
||||
if (fs.existsSync(profileDir) || fs.existsSync(rulesDir)) {
|
||||
installedProfiles.push(profileName);
|
||||
}
|
||||
}
|
||||
|
||||
return installedProfiles;
|
||||
}
|
||||
|
||||
/**
|
||||
* Check if removing the specified profiles would result in no rules profiles remaining
|
||||
* @param {string} projectRoot - Project root directory
|
||||
* @param {string[]} profilesToRemove - Array of profile names to remove
|
||||
* @returns {boolean} True if removal would result in no profiles remaining
|
||||
*/
|
||||
export function wouldRemovalLeaveNoProfiles(projectRoot, profilesToRemove) {
|
||||
const installedProfiles = getInstalledRulesProfiles(projectRoot);
|
||||
const remainingProfiles = installedProfiles.filter(
|
||||
(profile) => !profilesToRemove.includes(profile)
|
||||
);
|
||||
|
||||
return remainingProfiles.length === 0 && installedProfiles.length > 0;
|
||||
}
|
||||
175
tests/unit/rules-safety-check.test.js
Normal file
175
tests/unit/rules-safety-check.test.js
Normal file
@@ -0,0 +1,175 @@
|
||||
import {
|
||||
getInstalledRulesProfiles,
|
||||
wouldRemovalLeaveNoProfiles
|
||||
} from '../../src/utils/rules-detection.js';
|
||||
import { rulesDirect } from '../../mcp-server/src/core/direct-functions/rules.js';
|
||||
import fs from 'fs';
|
||||
import path from 'path';
|
||||
import { jest } from '@jest/globals';
|
||||
|
||||
// Mock logger
|
||||
const mockLog = {
|
||||
info: jest.fn(),
|
||||
error: jest.fn(),
|
||||
debug: jest.fn()
|
||||
};
|
||||
|
||||
describe('Rules Safety Check', () => {
|
||||
let mockExistsSync;
|
||||
let mockRmSync;
|
||||
let mockReaddirSync;
|
||||
|
||||
beforeEach(() => {
|
||||
jest.clearAllMocks();
|
||||
|
||||
// Set up spies on fs methods
|
||||
mockExistsSync = jest.spyOn(fs, 'existsSync');
|
||||
mockRmSync = jest.spyOn(fs, 'rmSync').mockImplementation(() => {});
|
||||
mockReaddirSync = jest.spyOn(fs, 'readdirSync').mockReturnValue([]);
|
||||
});
|
||||
|
||||
afterEach(() => {
|
||||
// Restore all mocked functions
|
||||
jest.restoreAllMocks();
|
||||
});
|
||||
|
||||
describe('getInstalledRulesProfiles', () => {
|
||||
it('should detect installed profiles correctly', () => {
|
||||
const projectRoot = '/test/project';
|
||||
|
||||
// Mock fs.existsSync to simulate installed profiles
|
||||
mockExistsSync.mockImplementation((filePath) => {
|
||||
if (filePath.includes('.cursor') || filePath.includes('.roo')) {
|
||||
return true;
|
||||
}
|
||||
return false;
|
||||
});
|
||||
|
||||
const installed = getInstalledRulesProfiles(projectRoot);
|
||||
expect(installed).toContain('cursor');
|
||||
expect(installed).toContain('roo');
|
||||
expect(installed).not.toContain('windsurf');
|
||||
expect(installed).not.toContain('cline');
|
||||
});
|
||||
|
||||
it('should return empty array when no profiles are installed', () => {
|
||||
const projectRoot = '/test/project';
|
||||
|
||||
// Mock fs.existsSync to return false for all paths
|
||||
mockExistsSync.mockReturnValue(false);
|
||||
|
||||
const installed = getInstalledRulesProfiles(projectRoot);
|
||||
expect(installed).toEqual([]);
|
||||
});
|
||||
});
|
||||
|
||||
describe('wouldRemovalLeaveNoProfiles', () => {
|
||||
it('should return true when removing all installed profiles', () => {
|
||||
const projectRoot = '/test/project';
|
||||
|
||||
// Mock fs.existsSync to simulate cursor and roo installed
|
||||
mockExistsSync.mockImplementation((filePath) => {
|
||||
return filePath.includes('.cursor') || filePath.includes('.roo');
|
||||
});
|
||||
|
||||
const result = wouldRemovalLeaveNoProfiles(projectRoot, [
|
||||
'cursor',
|
||||
'roo'
|
||||
]);
|
||||
expect(result).toBe(true);
|
||||
});
|
||||
|
||||
it('should return false when removing only some profiles', () => {
|
||||
const projectRoot = '/test/project';
|
||||
|
||||
// Mock fs.existsSync to simulate cursor and roo installed
|
||||
mockExistsSync.mockImplementation((filePath) => {
|
||||
return filePath.includes('.cursor') || filePath.includes('.roo');
|
||||
});
|
||||
|
||||
const result = wouldRemovalLeaveNoProfiles(projectRoot, ['roo']);
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
|
||||
it('should return false when no profiles are currently installed', () => {
|
||||
const projectRoot = '/test/project';
|
||||
|
||||
// Mock fs.existsSync to return false for all paths
|
||||
mockExistsSync.mockReturnValue(false);
|
||||
|
||||
const result = wouldRemovalLeaveNoProfiles(projectRoot, ['cursor']);
|
||||
expect(result).toBe(false);
|
||||
});
|
||||
});
|
||||
|
||||
describe('MCP Safety Check Integration', () => {
|
||||
it('should block removal of all profiles without force', async () => {
|
||||
const projectRoot = '/test/project';
|
||||
|
||||
// Mock fs.existsSync to simulate installed profiles
|
||||
mockExistsSync.mockImplementation((filePath) => {
|
||||
return filePath.includes('.cursor') || filePath.includes('.roo');
|
||||
});
|
||||
|
||||
const result = await rulesDirect(
|
||||
{
|
||||
action: 'remove',
|
||||
profiles: ['cursor', 'roo'],
|
||||
projectRoot,
|
||||
force: false
|
||||
},
|
||||
mockLog
|
||||
);
|
||||
|
||||
expect(result.success).toBe(false);
|
||||
expect(result.error.code).toBe('CRITICAL_REMOVAL_BLOCKED');
|
||||
expect(result.error.message).toContain('CRITICAL');
|
||||
});
|
||||
|
||||
it('should allow removal of all profiles with force', async () => {
|
||||
const projectRoot = '/test/project';
|
||||
|
||||
// Mock fs.existsSync and other file operations for successful removal
|
||||
mockExistsSync.mockReturnValue(true);
|
||||
|
||||
const result = await rulesDirect(
|
||||
{
|
||||
action: 'remove',
|
||||
profiles: ['cursor', 'roo'],
|
||||
projectRoot,
|
||||
force: true
|
||||
},
|
||||
mockLog
|
||||
);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(result.data).toBeDefined();
|
||||
});
|
||||
|
||||
it('should allow partial removal without force', async () => {
|
||||
const projectRoot = '/test/project';
|
||||
|
||||
// Mock fs.existsSync to simulate multiple profiles installed
|
||||
mockExistsSync.mockImplementation((filePath) => {
|
||||
return (
|
||||
filePath.includes('.cursor') ||
|
||||
filePath.includes('.roo') ||
|
||||
filePath.includes('.windsurf')
|
||||
);
|
||||
});
|
||||
|
||||
const result = await rulesDirect(
|
||||
{
|
||||
action: 'remove',
|
||||
profiles: ['roo'], // Only removing one profile
|
||||
projectRoot,
|
||||
force: false
|
||||
},
|
||||
mockLog
|
||||
);
|
||||
|
||||
expect(result.success).toBe(true);
|
||||
expect(result.data).toBeDefined();
|
||||
});
|
||||
});
|
||||
});
|
||||
Reference in New Issue
Block a user