diff --git a/.changeset/shared-ide-restart-hint.md b/.changeset/shared-ide-restart-hint.md new file mode 100644 index 0000000000..0e42e87b6e --- /dev/null +++ b/.changeset/shared-ide-restart-hint.md @@ -0,0 +1,5 @@ +--- +"@fission-ai/openspec": patch +--- + +`openspec init` and `openspec update` now share the IDE restart hint: "Restart your IDE to refresh commands." or "Restart your IDE to refresh skills." The message also covers removing workflows, without claiming that new files were generated. diff --git a/src/core/init.ts b/src/core/init.ts index 2d098cb9e7..7707f749d7 100644 --- a/src/core/init.ts +++ b/src/core/init.ts @@ -55,6 +55,7 @@ import { resolveToolSkillsDir, toolSupportsSkills, type ToolSkillStatus, + formatIdeRestart, } from './shared/index.js'; import { getGlobalConfig, type Delivery, type Profile } from './global-config.js'; import { getProfileWorkflows, CORE_WORKFLOWS, ALL_WORKFLOWS } from './profiles.js'; @@ -148,7 +149,6 @@ type ValidatedInitTool = { skillsRoot: string; isGlobalSkillTarget: boolean; wasConfigured: boolean; - requiresIdeRestart?: boolean; writesSkills: boolean; }; @@ -843,7 +843,6 @@ export class InitCommand { skillsRoot: isGlobalSkillTarget ? skillsPath : projectPath, isGlobalSkillTarget, wasConfigured: preState?.configured ?? false, - requiresIdeRestart: tool.requiresIdeRestart, writesSkills: !tool.skillsDir || skillWriters.has(tool.value), }); } @@ -1402,37 +1401,16 @@ export class InitCommand { console.log(`Learn more: ${chalk.cyan('https://github.com/Fission-AI/OpenSpec')}`); console.log(`Feedback: ${chalk.cyan('https://github.com/Fission-AI/OpenSpec/issues')}`); - // Restart instruction only when at least one IDE/editor-resident tool - // actually received a generated surface. Two conditions, coupled to the SAME - // tool: (1) its commands/skills are loaded by a long-running editor process - // (CLI tools pick the files up immediately, so a restart line would be wrong - // for them — see #1067), and (2) a surface was actually generated for it - // under the active delivery (an IDE tool that generated nothing has nothing a - // restart would pick up, even if a co-configured CLI tool did generate). - // Wording follows what the IDE tool itself generated, not the global - // aggregate: it must not say "commands" when the IDE tool only got skills - // while a co-configured CLI tool got commands. Not "slash commands" either: - // Amazon Q's generated files are prompt-library entries invoked with @, so a - // restart line promising slash commands would be wrong for it. - const restartCommandsGenerated = successfulTools.some( - (tool) => - tool.requiresIdeRestart && - shouldGenerateCommandsForTool(tool.value, activeDelivery) - ); - const restartSkillsGenerated = successfulTools.some( - (tool) => - tool.requiresIdeRestart && - shouldGenerateSkillsForTool(tool.value, activeDelivery) + // Restart instruction for successfully configured IDE/editor-resident tools + // with a supported surface under the active delivery. The rule and wording live in + // formatIdeRestart so `update` says the same thing for the same event. + const restartHint = formatIdeRestart( + successfulTools.map((tool) => tool.value), + activeDelivery ); - if (restartCommandsGenerated || restartSkillsGenerated) { + if (restartHint) { console.log(); - console.log( - chalk.white( - restartCommandsGenerated - ? 'Restart your IDE for the new commands to take effect.' - : 'Restart your IDE for the new skills to take effect.' - ) - ); + console.log(chalk.white(restartHint)); } console.log(); diff --git a/src/core/shared/ide-restart.ts b/src/core/shared/ide-restart.ts new file mode 100644 index 0000000000..2a5db017b2 --- /dev/null +++ b/src/core/shared/ide-restart.ts @@ -0,0 +1,63 @@ +/** + * IDE restart hint + * + * Shared restart guidance for tools successfully configured by init or update. + * The wording covers additions, updates, and removals, including an empty + * workflow selection that removes every generated file. + */ + +import { AI_TOOLS } from '../config.js'; +import { + shouldGenerateCommandsForTool, + shouldGenerateSkillsForTool, +} from '../command-surface.js'; +import type { Delivery } from '../global-config.js'; + +/** The surface a restart hint names. Absent when no hint is due. */ +export type IdeRestartSurface = 'commands' | 'skills'; + +function isIdeResident(toolId: string): boolean { + return Boolean( + AI_TOOLS.find((tool) => tool.value === toolId)?.requiresIdeRestart + ); +} + +/** + * Both conditions stay coupled to the SAME tool: its surfaces are loaded by a + * long-running editor process (a CLI picks them up immediately, so a restart + * line would be wrong for it — see #1067), and it supports a generated surface + * under the active delivery. A CLI tool's commands must not determine the hint + * for an IDE tool that only supports skills. Commands take precedence when + * both surfaces are supported under the active delivery. + */ +export function resolveIdeRestartSurface( + toolIds: readonly string[], + delivery: Delivery +): IdeRestartSurface | null { + const ideTools = [...new Set(toolIds)].filter(isIdeResident); + + if (ideTools.some((toolId) => shouldGenerateCommandsForTool(toolId, delivery))) { + return 'commands'; + } + + if (ideTools.some((toolId) => shouldGenerateSkillsForTool(toolId, delivery))) { + return 'skills'; + } + + return null; +} + +/** + * The restart line to print, or null when no restart is needed. Deliberately + * not "slash commands": Amazon Q's generated files are prompt-library entries + * invoked with `@`, so promising slash commands would be wrong for it. + */ +export function formatIdeRestart( + toolIds: readonly string[], + delivery: Delivery +): string | null { + const surface = resolveIdeRestartSurface(toolIds, delivery); + return surface + ? `Restart your IDE to refresh ${surface}.` + : null; +} diff --git a/src/core/shared/index.ts b/src/core/shared/index.ts index 53533b6907..fc7c1cd547 100644 --- a/src/core/shared/index.ts +++ b/src/core/shared/index.ts @@ -36,3 +36,9 @@ export { hasGlobalSkillTarget, resolveToolSkillsDir, } from './skill-paths.js'; + +export { + type IdeRestartSurface, + resolveIdeRestartSurface, + formatIdeRestart, +} from './ide-restart.js'; diff --git a/src/core/update.ts b/src/core/update.ts index 4af31cd2f4..ccbc9b24b0 100644 --- a/src/core/update.ts +++ b/src/core/update.ts @@ -27,6 +27,7 @@ import { resolveToolSkillsDir, toolSupportsSkills, type ToolVersionStatus, + formatIdeRestart, } from './shared/index.js'; import { detectLegacyArtifacts, @@ -501,18 +502,9 @@ export class UpdateCommand { console.log(); const affectedToolIds = [...new Set([...newlyConfiguredTools, ...updatedToolIds])]; - const shouldRestartIde = affectedToolIds.some((toolId) => { - const tool = AI_TOOLS.find((candidate) => candidate.value === toolId); - return Boolean( - tool?.requiresIdeRestart && - ( - shouldGenerateCommandsForTool(toolId, delivery) || - shouldGenerateSkillsForTool(toolId, delivery) - ) - ); - }); - if (shouldRestartIde) { - console.log(chalk.dim('Restart your IDE for changes to take effect.')); + const restartHint = formatIdeRestart(affectedToolIds, delivery); + if (restartHint) { + console.log(chalk.dim(restartHint)); } if (failedTools.length > 0) { throw new Error(`OpenSpec update failed for: ${failedTools.map((tool) => tool.name).join(', ')}`); diff --git a/test/core/init.test.ts b/test/core/init.test.ts index 3c20733671..43a03d77d5 100644 --- a/test/core/init.test.ts +++ b/test/core/init.test.ts @@ -1067,7 +1067,7 @@ describe('InitCommand', () => { await initCommand.execute(testDir); - expect(getConsoleOutput()).toContain('Restart your IDE for the new commands to take effect.'); + expect(getConsoleOutput()).toContain('Restart your IDE to refresh commands.'); }); it('should word the restart hint for skills when an IDE tool gets only a skill surface', async () => { @@ -1078,7 +1078,7 @@ describe('InitCommand', () => { await initCommand.execute(testDir); - expect(getConsoleOutput()).toContain('Restart your IDE for the new skills to take effect.'); + expect(getConsoleOutput()).toContain('Restart your IDE to refresh skills.'); }); it('should create skills for multiple tools at once', async () => { @@ -2099,7 +2099,7 @@ describe('InitCommand - profile and detection features', () => { // Commands were generated, but they are not slash commands. const restartHint = logCalls.find((entry) => entry.includes('Restart your IDE')); - expect(restartHint).toContain('Restart your IDE for the new commands to take effect.'); + expect(restartHint).toContain('Restart your IDE to refresh commands.'); expect(restartHint).not.toContain('slash commands'); }); diff --git a/test/core/shared-ide-restart.test.ts b/test/core/shared-ide-restart.test.ts new file mode 100644 index 0000000000..118a2e06c6 --- /dev/null +++ b/test/core/shared-ide-restart.test.ts @@ -0,0 +1,62 @@ +import { afterEach, describe, expect, it, vi } from 'vitest'; + +import { CommandAdapterRegistry } from '../../src/core/command-generation/index.js'; + +import { + formatIdeRestart, + resolveIdeRestartSurface, +} from '../../src/core/shared/ide-restart.js'; + +describe('resolveIdeRestartSurface', () => { + afterEach(() => vi.restoreAllMocks()); + + it('names commands when an IDE-resident tool received command files', () => { + expect(resolveIdeRestartSurface(['cursor'], 'both')).toBe('commands'); + expect(resolveIdeRestartSurface(['cursor'], 'commands')).toBe('commands'); + }); + + it('names skills when the IDE-resident tool only received skills', () => { + expect(resolveIdeRestartSurface(['cursor'], 'skills')).toBe('skills'); + }); + + it('stays silent for CLI-resident tools, which pick files up immediately', () => { + expect(resolveIdeRestartSurface(['claude'], 'both')).toBeNull(); + expect(resolveIdeRestartSurface(['codex'], 'skills')).toBeNull(); + }); + + it.each([ + ['commands', null], + ['both', 'skills'], + ] as const)('does not borrow CLI commands when delivery is %s', (delivery, expected) => { + // Model an IDE tool without an adapter: it receives no files with commands + // delivery, and only skills with both. Claude still receives commands. + const hasAdapter = CommandAdapterRegistry.has.bind(CommandAdapterRegistry); + vi.spyOn(CommandAdapterRegistry, 'has').mockImplementation( + (toolId) => toolId !== 'cursor' && hasAdapter(toolId) + ); + + expect(resolveIdeRestartSurface(['claude', 'cursor'], delivery)).toBe(expected); + }); + + it('handles duplicates and empty input', () => { + expect(resolveIdeRestartSurface(['cursor', 'cursor', 'claude'], 'commands')).toBe( + 'commands' + ); + expect(resolveIdeRestartSurface([], 'both')).toBeNull(); + }); +}); + +describe('formatIdeRestart', () => { + it('produces the same sentence init and update both print', () => { + expect(formatIdeRestart(['cursor'], 'both')).toBe( + 'Restart your IDE to refresh commands.' + ); + expect(formatIdeRestart(['cursor'], 'skills')).toBe( + 'Restart your IDE to refresh skills.' + ); + }); + + it('returns null when no restart is needed', () => { + expect(formatIdeRestart(['claude'], 'both')).toBeNull(); + }); +}); diff --git a/test/core/update.test.ts b/test/core/update.test.ts index a8e2634a11..db193b3d22 100644 --- a/test/core/update.test.ts +++ b/test/core/update.test.ts @@ -1947,7 +1947,12 @@ metadata: consoleSpy.mockRestore(); }); - it('should suggest an IDE restart for IDE-resident tools', async () => { + it.each([ + ['both', 'commands'], + ['commands', 'commands'], + ['skills', 'skills'], + ] as const)('should name the generated IDE surface with %s delivery', async (delivery, surface) => { + setMockConfig({ featureFlags: {}, profile: 'core', delivery }); const skillsDir = path.join(testDir, '.cursor', 'skills'); await fs.mkdir(path.join(skillsDir, 'openspec-explore'), { recursive: true, @@ -1962,11 +1967,64 @@ metadata: await updateCommand.execute(testDir); expect(consoleSpy).toHaveBeenCalledWith( - expect.stringContaining('Restart your IDE') + expect.stringContaining(`Restart your IDE to refresh ${surface}.`) ); + expect(await FileSystemUtils.fileExists( + path.join(testDir, '.cursor', 'commands', 'opsx-explore.md') + )).toBe(delivery !== 'skills'); + expect(await FileSystemUtils.fileExists( + path.join(skillsDir, 'openspec-explore', 'SKILL.md') + )).toBe(delivery !== 'commands'); + + consoleSpy.mockClear(); + await updateCommand.execute(testDir); + + expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('up to date')); + expect(consoleSpy).not.toHaveBeenCalledWith(expect.stringContaining('Restart your IDE')); consoleSpy.mockRestore(); }); + + it.each(['both', 'commands', 'skills'] as const)( + 'should describe removal-only IDE updates with %s delivery', + async (delivery) => { + setMockConfig({ featureFlags: {}, profile: 'core', delivery }); + await new InitCommand({ tools: 'cursor', force: true }).execute(testDir); + setMockConfig({ featureFlags: {}, profile: 'custom', workflows: [], delivery }); + const consoleSpy = vi.spyOn(console, 'log'); + + await updateCommand.execute(testDir); + + expect(await FileSystemUtils.fileExists( + path.join(testDir, '.cursor', 'commands', 'opsx-explore.md') + )).toBe(false); + expect(await FileSystemUtils.fileExists( + path.join(testDir, '.cursor', 'skills', 'openspec-explore', 'SKILL.md') + )).toBe(false); + const surface = delivery === 'skills' ? 'skills' : 'commands'; + expect(consoleSpy).toHaveBeenCalledWith( + expect.stringContaining(`Restart your IDE to refresh ${surface}.`) + ); + expect(consoleSpy).not.toHaveBeenCalledWith( + expect.stringContaining('Restart your IDE for the new') + ); + } + ); + + it('should not suggest an IDE restart when only a CLI tool needs updating', async () => { + await new InitCommand({ tools: 'claude,cursor', force: true }).execute(testDir); + await fs.writeFile( + path.join(testDir, '.claude', 'skills', 'openspec-explore', 'SKILL.md'), + 'old' + ); + const consoleSpy = vi.spyOn(console, 'log'); + + await updateCommand.execute(testDir); + + expect(consoleSpy).toHaveBeenCalledWith(expect.stringContaining('Updated: Claude Code')); + expect(consoleSpy).not.toHaveBeenCalledWith(expect.stringContaining('Updated: Cursor')); + expect(consoleSpy).not.toHaveBeenCalledWith(expect.stringContaining('Restart your IDE')); + }); }); describe('smart update detection', () => { @@ -2570,7 +2628,13 @@ ${OPENSPEC_MARKERS.end} expect(menuLines).toHaveLength(1); expect(menuLines[0]).toContain('/opsx-propose'); expect(logCalls.some((entry) => entry.includes('/opsx:propose'))).toBe(false); - expect(logCalls.some((entry) => entry.includes('Restart your IDE'))).toBe(true); + // The hint names what was generated, the same sentence init prints, rather + // than update's older generic "changes". + expect( + logCalls.some((entry) => + entry.includes('Restart your IDE to refresh commands.') + ) + ).toBe(true); }); it('should preserve legacy Codex prompts when a configured Codex tool lacks the replacement workflow', async () => {