diff --git a/openspec/changes/simplify-skill-installation/specs/cli-update/spec.md b/openspec/changes/simplify-skill-installation/specs/cli-update/spec.md index c38d268292..3ee636dec3 100644 --- a/openspec/changes/simplify-skill-installation/specs/cli-update/spec.md +++ b/openspec/changes/simplify-skill-installation/specs/cli-update/spec.md @@ -160,15 +160,14 @@ The update command SHALL only run inside an initialized OpenSpec project. - **THEN** the system SHALL display: "No OpenSpec project found. Run 'openspec init' to set up." - **THEN** the system SHALL exit with code 1 -### Requirement: Extra workflows preserved -The update command SHALL NOT remove workflow files that aren't in the current profile. +### Requirement: Extra workflows synchronized to active profile +The update command SHALL remove workflow files that are no longer selected in the current profile. -#### Scenario: Extra workflows from previous profile +#### Scenario: Deselected workflows from previous profile - **WHEN** user runs `openspec update` -- **AND** project has workflows not in current profile (e.g., user switched from custom to core) -- **THEN** the system SHALL NOT delete those extra workflow files -- **THEN** the system SHALL only add/update workflows in the current profile -- **THEN** the system SHALL display a note: "Note: extra workflows not in profile (use `openspec config profile` to manage)" +- **AND** project has workflows not in current profile (e.g., user switched from custom to core or deselected workflows via `openspec config profile`) +- **THEN** the system SHALL delete skill and command workflow files for deselected workflows (respecting active delivery mode) +- **THEN** the system SHALL keep only workflows currently selected in profile #### Scenario: Delivery change with extra workflows - **WHEN** user runs `openspec update` diff --git a/src/core/profile-sync-drift.ts b/src/core/profile-sync-drift.ts index d615d818d6..782bdcc9fa 100644 --- a/src/core/profile-sync-drift.ts +++ b/src/core/profile-sync-drift.ts @@ -80,11 +80,10 @@ export function getConfiguredToolsForProfileSync(projectPath: string): string[] /** * Detects if a single tool has profile/delivery drift against the desired state. * - * Note: this function is intentionally scoped to "required artifacts missing" - * and "artifacts that should not exist for the selected delivery mode". - * Extra workflows that are outside the desired profile are handled by - * `hasProjectConfigDrift`, which compares installed workflow IDs against - * the desired workflow set. + * This function covers: + * - required artifacts missing for selected workflows + * - artifacts that should not exist for the selected delivery mode + * - artifacts for workflows that were deselected from the current profile */ export function hasToolProfileOrDeliveryDrift( projectPath: string, @@ -96,6 +95,7 @@ export function hasToolProfileOrDeliveryDrift( if (!tool?.skillsDir) return false; const knownDesiredWorkflows = toKnownWorkflows(desiredWorkflows); + const desiredWorkflowSet = new Set(knownDesiredWorkflows); const skillsDir = path.join(projectPath, tool.skillsDir, 'skills'); const adapter = CommandAdapterRegistry.get(toolId); const shouldGenerateSkills = delivery !== 'commands'; @@ -109,6 +109,16 @@ export function hasToolProfileOrDeliveryDrift( return true; } } + + // Deselecting workflows in a profile should trigger sync. + for (const workflow of ALL_WORKFLOWS) { + if (desiredWorkflowSet.has(workflow)) continue; + const dirName = WORKFLOW_TO_SKILL_DIR[workflow]; + const skillDir = path.join(skillsDir, dirName); + if (fs.existsSync(skillDir)) { + return true; + } + } } else { for (const workflow of ALL_WORKFLOWS) { const dirName = WORKFLOW_TO_SKILL_DIR[workflow]; @@ -127,6 +137,16 @@ export function hasToolProfileOrDeliveryDrift( return true; } } + + // Deselecting workflows in a profile should trigger sync. + for (const workflow of ALL_WORKFLOWS) { + if (desiredWorkflowSet.has(workflow)) continue; + const cmdPath = adapter.getFilePath(workflow); + const fullPath = path.isAbsolute(cmdPath) ? cmdPath : path.join(projectPath, cmdPath); + if (fs.existsSync(fullPath)) { + return true; + } + } } else if (!shouldGenerateCommands && adapter) { for (const workflow of ALL_WORKFLOWS) { const cmdPath = adapter.getFilePath(workflow); diff --git a/src/core/update.ts b/src/core/update.ts index 87ddb73acb..62db8a08fd 100644 --- a/src/core/update.ts +++ b/src/core/update.ts @@ -176,6 +176,8 @@ export class UpdateCommand { const failedTools: Array<{ name: string; error: string }> = []; let removedCommandCount = 0; let removedSkillCount = 0; + let removedDeselectedCommandCount = 0; + let removedDeselectedSkillCount = 0; for (const toolId of toolsToUpdate) { const tool = AI_TOOLS.find((t) => t.value === toolId); @@ -197,6 +199,8 @@ export class UpdateCommand { const skillContent = generateSkillContent(template, OPENSPEC_VERSION, transformer); await FileSystemUtils.writeFile(skillFile, skillContent); } + + removedDeselectedSkillCount += await this.removeUnselectedSkillDirs(skillsDir, desiredWorkflows); } // Delete skill directories if delivery is commands-only @@ -214,6 +218,12 @@ export class UpdateCommand { const commandFile = path.isAbsolute(cmd.path) ? cmd.path : path.join(resolvedProjectPath, cmd.path); await FileSystemUtils.writeFile(commandFile, cmd.fileContent); } + + removedDeselectedCommandCount += await this.removeUnselectedCommandFiles( + resolvedProjectPath, + toolId, + desiredWorkflows + ); } } @@ -247,6 +257,12 @@ export class UpdateCommand { if (removedSkillCount > 0) { console.log(chalk.dim(`Removed: ${removedSkillCount} skill directories (delivery: commands)`)); } + if (removedDeselectedCommandCount > 0) { + console.log(chalk.dim(`Removed: ${removedDeselectedCommandCount} command files (deselected workflows)`)); + } + if (removedDeselectedSkillCount > 0) { + console.log(chalk.dim(`Removed: ${removedDeselectedSkillCount} skill directories (deselected workflows)`)); + } // 12. Show onboarding message for newly configured tools from legacy upgrade if (newlyConfiguredTools.length > 0) { @@ -378,6 +394,36 @@ export class UpdateCommand { return removed; } + /** + * Removes skill directories for workflows that are no longer selected in the active profile. + * Returns the number of directories removed. + */ + private async removeUnselectedSkillDirs( + skillsDir: string, + desiredWorkflows: readonly (typeof ALL_WORKFLOWS)[number][] + ): Promise { + const desiredSet = new Set(desiredWorkflows); + let removed = 0; + + for (const workflow of ALL_WORKFLOWS) { + if (desiredSet.has(workflow)) continue; + const dirName = WORKFLOW_TO_SKILL_DIR[workflow]; + if (!dirName) continue; + + const skillDir = path.join(skillsDir, dirName); + try { + if (fs.existsSync(skillDir)) { + await fs.promises.rm(skillDir, { recursive: true, force: true }); + removed++; + } + } catch { + // Ignore errors + } + } + + return removed; + } + /** * Removes command files for workflows when delivery changed to skills-only. * Returns the number of files removed. @@ -408,6 +454,40 @@ export class UpdateCommand { return removed; } + /** + * Removes command files for workflows that are no longer selected in the active profile. + * Returns the number of files removed. + */ + private async removeUnselectedCommandFiles( + projectPath: string, + toolId: string, + desiredWorkflows: readonly (typeof ALL_WORKFLOWS)[number][] + ): Promise { + let removed = 0; + + const adapter = CommandAdapterRegistry.get(toolId); + if (!adapter) return 0; + + const desiredSet = new Set(desiredWorkflows); + + for (const workflow of ALL_WORKFLOWS) { + if (desiredSet.has(workflow)) continue; + const cmdPath = adapter.getFilePath(workflow); + const fullPath = path.isAbsolute(cmdPath) ? cmdPath : path.join(projectPath, cmdPath); + + try { + if (fs.existsSync(fullPath)) { + await fs.promises.unlink(fullPath); + removed++; + } + } catch { + // Ignore errors + } + } + + return removed; + } + /** * Detect and handle legacy OpenSpec artifacts. * Unlike init, update warns but continues if legacy files found in non-interactive mode. diff --git a/test/core/update.test.ts b/test/core/update.test.ts index a916967c9f..c36ac3da83 100644 --- a/test/core/update.test.ts +++ b/test/core/update.test.ts @@ -1567,7 +1567,7 @@ content consoleSpy.mockRestore(); }); - it('should display extra workflows note when workflows outside profile exist', async () => { + it('should remove workflows outside profile during update sync', async () => { // Set core profile (propose, explore, apply, archive) setMockConfig({ featureFlags: {}, @@ -1583,19 +1583,28 @@ content // Add a non-core workflow await fs.mkdir(path.join(skillsDir, 'openspec-new-change'), { recursive: true }); await fs.writeFile(path.join(skillsDir, 'openspec-new-change', 'SKILL.md'), 'old'); + const extraCommandFile = path.join(testDir, '.claude', 'commands', 'opsx', 'new.md'); + await fs.mkdir(path.dirname(extraCommandFile), { recursive: true }); + await fs.writeFile(extraCommandFile, 'old'); const consoleSpy = vi.spyOn(console, 'log'); await updateCommand.execute(testDir); - // Should display note about extra workflows + // Deselected workflow artifacts should be removed for both delivery surfaces. + expect(await FileSystemUtils.fileExists( + path.join(skillsDir, 'openspec-new-change', 'SKILL.md') + )).toBe(false); + expect(await FileSystemUtils.fileExists(extraCommandFile)).toBe(false); + + // Should report deselected workflow cleanup. const calls = consoleSpy.mock.calls.map(call => call.map(arg => String(arg)).join(' ') ); - const hasExtraNote = calls.some(call => - call.includes('extra workflows not in profile') + const hasDeselectedRemovalNote = calls.some(call => + call.includes('deselected workflows') ); - expect(hasExtraNote).toBe(true); + expect(hasDeselectedRemovalNote).toBe(true); consoleSpy.mockRestore(); });