-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: automatically switch to imported mode after successful import (#6491) #6507
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
efaaed9
57d3fbe
ef61905
f5a51c4
bcbf329
80413c0
ab10140
39c5cf7
00a0b63
080b61b
7a5ad14
2c73ff2
05ccf57
fdb1f35
10ce509
ab1f9fc
74fd8b4
6745c8f
faf2ee5
b2dadf9
f648e4c
a6d1e60
be90907
ed3a077
856313f
4dd68ea
b10fa5e
f016d7b
23855f2
3803c29
0a70720
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -11,6 +11,7 @@ import { | |
| type ProviderSettings, | ||
| type GlobalState, | ||
| type ClineMessage, | ||
| type ModeConfig, | ||
| TelemetryEventName, | ||
| } from "@roo-code/types" | ||
| import { CloudService } from "@roo-code/cloud" | ||
|
|
@@ -1876,12 +1877,47 @@ export const webviewMessageHandler = async ( | |
| // Update state after importing | ||
| const customModes = await provider.customModesManager.getCustomModes() | ||
| await updateGlobalState("customModes", customModes) | ||
| await provider.postStateToWebview() | ||
|
|
||
| // Send success message to webview | ||
| // Switch to the first imported mode if any were imported | ||
| if (result.importedModes && result.importedModes.length > 0) { | ||
| try { | ||
| // Switch to the first imported mode | ||
| const modeToSwitch = result.importedModes[0] | ||
|
|
||
| // Validate the mode exists before switching | ||
| const allModes = await provider.customModesManager.getCustomModes() | ||
| const validMode = allModes.find((m: ModeConfig) => m.slug === modeToSwitch) | ||
|
|
||
| if (validMode) { | ||
| // Validate that the mode slug is a valid string before type assertion | ||
| if (typeof modeToSwitch === "string" && modeToSwitch.length > 0) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The mode slug validation uses type assertion without proper type guards. Consider adding a type guard function for safer validation: |
||
| await provider.handleModeSwitch(modeToSwitch as Mode) | ||
| // Update the webview with the new mode | ||
| await provider.postStateToWebview() | ||
|
|
||
| // Track telemetry for automatic mode switch after import | ||
| TelemetryService.instance.captureEvent(TelemetryEventName.MODE_SWITCH, { | ||
| taskId: provider.getCurrentCline()?.taskId || "no-task", | ||
| newMode: modeToSwitch, | ||
| trigger: "auto-import", | ||
| }) | ||
| } else { | ||
| provider.log(`Invalid mode slug for switching: ${modeToSwitch}`) | ||
| } | ||
| } | ||
| } catch (error) { | ||
| provider.log( | ||
| `Failed to switch to imported mode: ${error instanceof Error ? error.message : String(error)}`, | ||
| ) | ||
| // Continue with success response - import succeeded even if switch failed | ||
| } | ||
| } | ||
|
|
||
| // Send success message to webview with imported modes info | ||
| provider.postMessageToWebview({ | ||
| type: "importModeResult", | ||
| success: true, | ||
| payload: { importedModes: result.importedModes }, | ||
| }) | ||
|
|
||
| // Show success message | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -460,10 +460,24 @@ const ModesView = ({ onDone }: ModesViewProps) => { | |
| setIsImporting(false) | ||
| setShowImportDialog(false) | ||
|
|
||
| if (!message.success) { | ||
| // Extract data from message payload | ||
| const { success, error } = message | ||
| const importedModes = message.payload?.importedModes | ||
|
|
||
| if (success && importedModes && importedModes.length > 0) { | ||
| // Find the imported mode configuration | ||
| const importedModeSlug = importedModes[0] | ||
| const importedMode = | ||
| findModeBySlug(importedModeSlug, customModes) || modes.find((m) => m.slug === importedModeSlug) | ||
|
|
||
| if (importedMode) { | ||
| // Switch to the imported mode | ||
| handleModeSwitch(importedMode) | ||
| } | ||
| } else if (!success) { | ||
| // Only log error if it's not a cancellation | ||
| if (message.error !== "cancelled") { | ||
| console.error("Failed to import mode:", message.error) | ||
| if (error !== "cancelled") { | ||
| console.error("Failed to import mode:", error) | ||
| } | ||
| } | ||
| } else if (message.type === "checkRulesDirectoryResult") { | ||
|
|
@@ -487,7 +501,8 @@ const ModesView = ({ onDone }: ModesViewProps) => { | |
|
|
||
| window.addEventListener("message", handler) | ||
| return () => window.removeEventListener("message", handler) | ||
| }, []) // Empty dependency array - only register once | ||
| // Re-register handler when dependencies change to ensure it has access to latest values | ||
| }, [customModes, findModeBySlug, handleModeSwitch, modes]) | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The component re-registers the message event handler when dependencies change, which could potentially cause race conditions if multiple imports happen quickly. Consider using a more stable event handling pattern, perhaps with a cleanup function that properly removes the previous handler before adding a new one, or using a ref to maintain handler stability. |
||
|
|
||
| const handleAgentReset = ( | ||
| modeSlug: string, | ||
|
|
||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Minor style inconsistency - this line uses double quotes while the rest of the file uses single quotes. Consider maintaining consistent quote style: