fix: only show 'Restart your IDE' message for IDE-embedded tools#1097
fix: only show 'Restart your IDE' message for IDE-embedded tools#1097mvanhorn wants to merge 2 commits into
Conversation
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughInit now marks certain tools as IDE-resident via a new requiresIdeRestart flag, threads that flag through validation and generation, and shows the IDE restart prompt only when a configured tool requires it; tests and the changeset were updated accordingly. ChangesIDE restart prompt refinement
Estimated code review effort🎯 3 (Moderate) | ⏱️ ~22 minutes Possibly related PRs
Poem
🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
src/core/init.ts (1)
635-647:⚠️ Potential issue | 🟠 Major | ⚡ Quick winType mismatch: parameter type must include
requiresIdeRestartfield.The
toolsparameter is typed asArray<{ value: string; name: string; skillsDir: string; wasConfigured: boolean }>, which lacks therequiresIdeRestartfield. However, at line 730, the code accessestool.requiresIdeRestarton objects derived fromresults.createdToolsandresults.refreshedTools, which are typed astypeof tools.This creates a type safety issue where the code accesses a property not present in the declared type. While it works at runtime (because the actual objects passed are
SelectedTool[]instances that include this field), it violates TypeScript's type guarantees.🔧 Suggested fix
Replace the inline anonymous type with
SelectedTool[]:private displaySuccessMessage( projectPath: string, - tools: Array<{ value: string; name: string; skillsDir: string; wasConfigured: boolean }>, + tools: SelectedTool[], results: { createdTools: typeof tools; refreshedTools: typeof tools;This ensures the parameter type accurately reflects the properties accessed in the method body and maintains type safety.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@src/core/init.ts` around lines 635 - 647, The tools parameter type for displaySuccessMessage is missing the requiresIdeRestart field, causing a type mismatch when accessing tool.requiresIdeRestart on results.createdTools and results.refreshedTools; update the parameter and related result types to use the existing SelectedTool type (e.g., change tools: Array<{...}> to tools: SelectedTool[] and ensure results.createdTools / results.refreshedTools are typed as SelectedTool[]), so the method signature accurately reflects the properties used inside displaySuccessMessage.
🧹 Nitpick comments (1)
test/core/init.test.ts (1)
195-209: ⚡ Quick winGood test coverage for the basic scenarios.
The tests correctly verify that CLI-only tools don't trigger the restart message while IDE-resident tools do. The test cases use appropriate representative tools (claude and cursor) that align with the configuration changes.
💡 Optional: Add test for mixed tool scenario
Consider adding a test case that configures both a CLI tool and an IDE-resident tool simultaneously to verify that the restart message is shown when at least one tool requires it:
it('should suggest an IDE restart when configuring mixed CLI and IDE tools', async () => { const initCommand = new InitCommand({ tools: 'claude,cursor', force: true }); await initCommand.execute(testDir); expect(getConsoleOutput()).toContain('Restart your IDE for slash commands to take effect.'); });This would provide additional confidence that the
.some()logic correctly handles mixed scenarios.🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In `@test/core/init.test.ts` around lines 195 - 209, Add a test that verifies mixed tool lists (CLI + IDE-resident) trigger the restart message: create an InitCommand with tools set to a combined string like 'claude,cursor' (or use the same constructor pattern as the existing tests) and call InitCommand.execute(testDir), then assert getConsoleOutput().toContain('Restart your IDE for slash commands to take effect.'); this ensures the logic in InitCommand.execute that checks tool types (.some() or equivalent) correctly shows the restart prompt when at least one IDE-resident tool (e.g., 'cursor') is present alongside a CLI-only tool (e.g., 'claude').
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Outside diff comments:
In `@src/core/init.ts`:
- Around line 635-647: The tools parameter type for displaySuccessMessage is
missing the requiresIdeRestart field, causing a type mismatch when accessing
tool.requiresIdeRestart on results.createdTools and results.refreshedTools;
update the parameter and related result types to use the existing SelectedTool
type (e.g., change tools: Array<{...}> to tools: SelectedTool[] and ensure
results.createdTools / results.refreshedTools are typed as SelectedTool[]), so
the method signature accurately reflects the properties used inside
displaySuccessMessage.
---
Nitpick comments:
In `@test/core/init.test.ts`:
- Around line 195-209: Add a test that verifies mixed tool lists (CLI +
IDE-resident) trigger the restart message: create an InitCommand with tools set
to a combined string like 'claude,cursor' (or use the same constructor pattern
as the existing tests) and call InitCommand.execute(testDir), then assert
getConsoleOutput().toContain('Restart your IDE for slash commands to take
effect.'); this ensures the logic in InitCommand.execute that checks tool types
(.some() or equivalent) correctly shows the restart prompt when at least one
IDE-resident tool (e.g., 'cursor') is present alongside a CLI-only tool (e.g.,
'claude').
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro
Run ID: 1c01042a-f725-4715-9f72-bb3b1ec0e98f
📒 Files selected for processing (4)
.changeset/restart-ide-hint.mdsrc/core/config.tssrc/core/init.tstest/core/init.test.ts
displaySuccessMessage accessed tool.requiresIdeRestart on objects derived from results.createdTools / results.refreshedTools, but the parameter type was an inline anonymous shape that didn't include the field. Use the existing SelectedTool type so the access is type-safe.
mvanhorn
force-pushed
the
fix/1067-openspec-restart-ide-message-conditional
branch
from
1365267 to
4c7c8ac
Compare
|
Addressed in 4c7c8ac:
|
1 participant
Summary
After
openspec init, the success screen always tells the user "Restart your IDE for slash commands to take effect." For CLI tools (Claude Code, Codex, Gemini CLI, etc.), there's no IDE to restart — the slash commands work as soon as the file exists. Add arequiresIdeRestartflag to each tool config; render the restart hint only for the tools where it actually applies (Cursor, Windsurf, Cline, Continue, RooCode, GitHub Copilot, etc.).Why this matters
#1067 reports the confusing always-on message. The fix annotates each
AIToolOptionwith a boolean and threads it throughvalidateToolsto the install flow, so the rendering knows which row needs the hint. Tools without an editor process (Claude Code, Codex, Crush, etc.) no longer surface the line.Fixes #1067
AI was used for assistance.
Summary by CodeRabbit
openspec initare now targeted: shown only when an IDE-resident tool (e.g., Cursor, GitHub Copilot, Continue, Windsurf) is configured, not for all tool setups.