From ca6f1514ddd5a1420ff506fba035189d9017dc46 Mon Sep 17 00:00:00 2001 From: devxpain <170700110+devxpain@users.noreply.github.com> Date: Tue, 27 May 2025 00:20:01 +0800 Subject: [PATCH 1/7] feat(custom-instructions): migrate custom instructions to file-based storage See discussion: https://github.com/RooCodeInc/Roo-Code/discussions/4000 Migrated the storage of "Custom Instructions for All Modes" from VS Code's global state to a dedicated file: `...Code/User/globalStorage/rooveterinaryinc.roo-cline/settings/custom_instructions.md`. Key changes include: - Implemented a migration utility to automatically move existing custom instructions from global state to the new file upon extension update. - Introduced new commands and UI buttons within the Prompts view to: - Open the `custom_instructions.md` file directly in the editor for easy editing. - Refresh the custom instructions in the UI by re-reading them from the file, ensuring changes made directly to the file are reflected. - The `updateCustomInstructions` function now writes/deletes the file instead of updating global state. - The initial state and refreshes now read custom instructions directly from the file. This change provides a more robust and user-friendly way to manage custom instructions, allowing for external editing and version control. --- src/core/webview/ClineProvider.ts | 57 ++++++++++++++++++- src/core/webview/webviewMessageHandler.ts | 7 +++ src/shared/WebviewMessage.ts | 2 + src/shared/globalFileNames.ts | 1 + src/utils/migrateSettings.ts | 15 +++++ webview-ui/src/components/modes/ModesView.tsx | 30 +++++++++- webview-ui/src/i18n/locales/en/prompts.json | 2 + 7 files changed, 109 insertions(+), 5 deletions(-) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 015e3ec184..31ccb383c6 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -989,11 +989,47 @@ export class ClineProvider } async updateCustomInstructions(instructions?: string) { - // User may be clearing the field. - await this.updateGlobalState("customInstructions", instructions || undefined) + const settingsDirPath = await this.ensureSettingsDirectoryExists() + const customInstructionsFilePath = path.join(settingsDirPath, GlobalFileNames.customInstructions) + + if (instructions && instructions.trim()) { + await fs.writeFile(customInstructionsFilePath, instructions.trim(), "utf-8") + } else { + // If instructions are empty or undefined, delete the file if it exists + try { + await fs.unlink(customInstructionsFilePath) + } catch (error) { + // Ignore if file doesn't exist + if ((error as NodeJS.ErrnoException).code !== "ENOENT") { + throw error + } + } + } await this.postStateToWebview() } + async openCustomInstructionsFile(): Promise { + const settingsDirPath = await this.ensureSettingsDirectoryExists() + const customInstructionsFilePath = path.join(settingsDirPath, GlobalFileNames.customInstructions) + const fileUri = vscode.Uri.file(customInstructionsFilePath) + await vscode.commands.executeCommand("vscode.open", fileUri, { preview: false, preserveFocus: true }) + await vscode.commands.executeCommand("workbench.action.files.revert", fileUri) + } + + async refreshCustomInstructions(): Promise { + const settingsDirPath = await this.ensureSettingsDirectoryExists() + const customInstructionsFilePath = path.join(settingsDirPath, GlobalFileNames.customInstructions) + let content: string | undefined = undefined + try { + content = await fs.readFile(customInstructionsFilePath, "utf-8") + } catch (error) { + if ((error as NodeJS.ErrnoException).code !== "ENOENT") { + throw error + } + } + await this.updateCustomInstructions(content) + } + // MCP async ensureMcpServersDirectoryExists(): Promise { @@ -1025,6 +1061,21 @@ export class ClineProvider return getSettingsDirectoryPath(globalStoragePath) } + private async readCustomInstructionsFromFile(): Promise { + const settingsDirPath = await this.ensureSettingsDirectoryExists() + const customInstructionsFilePath = path.join(settingsDirPath, GlobalFileNames.customInstructions) + + if (await fileExistsAtPath(customInstructionsFilePath)) { + try { + return await fs.readFile(customInstructionsFilePath, "utf-8") + } catch (error) { + this.log(`Error reading custom instructions file: ${error}`) + return undefined + } + } + return undefined + } + // OpenRouter async handleOpenRouterCallback(code: string) { @@ -1474,7 +1525,7 @@ export class ClineProvider return { apiConfiguration: providerSettings, lastShownAnnouncementId: stateValues.lastShownAnnouncementId, - customInstructions: stateValues.customInstructions, + customInstructions: await this.readCustomInstructionsFromFile(), apiModelId: stateValues.apiModelId, alwaysAllowReadOnly: stateValues.alwaysAllowReadOnly ?? false, alwaysAllowReadOnlyOutsideWorkspace: stateValues.alwaysAllowReadOnlyOutsideWorkspace ?? false, diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index 659d60f31a..66dae30eef 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -16,6 +16,7 @@ import { supportPrompt } from "../../shared/support-prompt" import { checkoutDiffPayloadSchema, checkoutRestorePayloadSchema, WebviewMessage } from "../../shared/WebviewMessage" import { checkExistKey } from "../../shared/checkExistApiConfig" import { experimentDefault } from "../../shared/experiments" +import { GlobalFileNames } from "../../shared/globalFileNames" import { Terminal } from "../../integrations/terminal/Terminal" import { openFile, openImage } from "../../integrations/misc/open-file" import { selectImages } from "../../integrations/misc/process-images" @@ -132,6 +133,12 @@ export const webviewMessageHandler = async (provider: ClineProvider, message: We case "customInstructions": await provider.updateCustomInstructions(message.text) break + case "openCustomInstructionsFile": + await provider.openCustomInstructionsFile() + break + case "refreshCustomInstructions": + await provider.refreshCustomInstructions() + break case "alwaysAllowReadOnly": await updateGlobalState("alwaysAllowReadOnly", message.bool ?? undefined) await provider.postStateToWebview() diff --git a/src/shared/WebviewMessage.ts b/src/shared/WebviewMessage.ts index 05136c18b3..9fd637e7a8 100644 --- a/src/shared/WebviewMessage.ts +++ b/src/shared/WebviewMessage.ts @@ -118,6 +118,8 @@ export interface WebviewMessage { | "deleteCustomMode" | "setopenAiCustomModelInfo" | "openCustomModesSettings" + | "openCustomInstructionsFile" + | "refreshCustomInstructions" | "checkpointDiff" | "checkpointRestore" | "deleteMcpServer" diff --git a/src/shared/globalFileNames.ts b/src/shared/globalFileNames.ts index 98b48485f0..f1192ae022 100644 --- a/src/shared/globalFileNames.ts +++ b/src/shared/globalFileNames.ts @@ -3,5 +3,6 @@ export const GlobalFileNames = { uiMessages: "ui_messages.json", mcpSettings: "mcp_settings.json", customModes: "custom_modes.yaml", + customInstructions: "custom_instructions.md", taskMetadata: "task_metadata.json", } diff --git a/src/utils/migrateSettings.ts b/src/utils/migrateSettings.ts index 406e5bd051..b02daf89b3 100644 --- a/src/utils/migrateSettings.ts +++ b/src/utils/migrateSettings.ts @@ -34,6 +34,21 @@ export async function migrateSettings( // Process each file migration try { + // Migrate custom instructions from GlobalState to file + const customInstructionsKey = "customInstructions" + const customInstructionsContent = context.globalState.get(customInstructionsKey) + const customInstructionsFilePath = path.join(settingsDir, GlobalFileNames.customInstructions) + + if (customInstructionsContent && !(await fileExistsAtPath(customInstructionsFilePath))) { + await fs.writeFile(customInstructionsFilePath, customInstructionsContent, "utf-8") + await context.globalState.update(customInstructionsKey, undefined) // Delete from GlobalState + outputChannel.appendLine("Migrated custom instructions from GlobalState to file.") + } else { + outputChannel.appendLine( + `Skipping custom instructions migration: ${customInstructionsContent ? "file already exists" : "no data in GlobalState"}`, + ) + } + for (const migration of fileMigrations) { const oldPath = path.join(settingsDir, migration.oldName) const newPath = path.join(settingsDir, migration.newName) diff --git a/webview-ui/src/components/modes/ModesView.tsx b/webview-ui/src/components/modes/ModesView.tsx index 18f4bdf3d2..a423b02fc2 100644 --- a/webview-ui/src/components/modes/ModesView.tsx +++ b/webview-ui/src/components/modes/ModesView.tsx @@ -1036,8 +1036,34 @@ const ModesView = ({ onDone }: ModesViewProps) => { -
-

{t("prompts:globalCustomInstructions.title")}

+
+
+

{t("prompts:globalCustomInstructions.title")}

+
+ + +
+
diff --git a/webview-ui/src/i18n/locales/en/prompts.json b/webview-ui/src/i18n/locales/en/prompts.json index f03d48ca92..20f534f56a 100644 --- a/webview-ui/src/i18n/locales/en/prompts.json +++ b/webview-ui/src/i18n/locales/en/prompts.json @@ -48,6 +48,8 @@ "globalCustomInstructions": { "title": "Custom Instructions for All Modes", "description": "These instructions apply to all modes. They provide a base set of behaviors that can be enhanced by mode-specific instructions below. <0>Learn more", + "openFile": "Open custom instructions file", + "refreshFile": "Refresh custom instructions from file", "loadFromFile": "Instructions can also be loaded from the .roo/rules/ folder in your workspace (.roorules and .clinerules are deprecated and will stop working soon)." }, "systemPrompt": { From 7aab01d83a56414cec1175340c045a1eac21d69a Mon Sep 17 00:00:00 2001 From: devxpain <170700110+devxpain@users.noreply.github.com> Date: Fri, 30 May 2025 21:10:12 +0800 Subject: [PATCH 2/7] refactor(fs): consolidate safeReadFile implementation - Moved `safeReadFile` function from multiple files to `src/utils/fs.ts` - Updated imports in affected files to use the centralized `safeReadFile` function - Simplified file reading logic by removing redundant implementations --- .../prompts/sections/custom-instructions.ts | 17 +------ .../prompts/sections/custom-system-prompt.ts | 19 +------- src/core/webview/ClineProvider.ts | 48 +++++++------------ src/utils/fs.ts | 16 +++++++ 4 files changed, 35 insertions(+), 65 deletions(-) diff --git a/src/core/prompts/sections/custom-instructions.ts b/src/core/prompts/sections/custom-instructions.ts index f9f4b7dea0..29c4a04d90 100644 --- a/src/core/prompts/sections/custom-instructions.ts +++ b/src/core/prompts/sections/custom-instructions.ts @@ -5,22 +5,7 @@ import { Dirent } from "fs" import { isLanguage } from "@roo-code/types" import { LANGUAGES } from "../../../shared/language" - -/** - * Safely read a file and return its trimmed content - */ -async function safeReadFile(filePath: string): Promise { - try { - const content = await fs.readFile(filePath, "utf-8") - return content.trim() - } catch (err) { - const errorCode = (err as NodeJS.ErrnoException).code - if (!errorCode || !["ENOENT", "EISDIR"].includes(errorCode)) { - throw err - } - return "" - } -} +import { safeReadFile } from "../../../utils/fs" /** * Check if a directory exists diff --git a/src/core/prompts/sections/custom-system-prompt.ts b/src/core/prompts/sections/custom-system-prompt.ts index f401000bb5..0d7de5a768 100644 --- a/src/core/prompts/sections/custom-system-prompt.ts +++ b/src/core/prompts/sections/custom-system-prompt.ts @@ -1,7 +1,7 @@ import fs from "fs/promises" import path from "path" import { Mode } from "../../../shared/modes" -import { fileExistsAtPath } from "../../../utils/fs" +import { fileExistsAtPath, safeReadFile } from "../../../utils/fs" export type PromptVariables = { workspace?: string @@ -25,23 +25,6 @@ function interpolatePromptContent(content: string, variables: PromptVariables): return interpolatedContent } -/** - * Safely reads a file, returning an empty string if the file doesn't exist - */ -async function safeReadFile(filePath: string): Promise { - try { - const content = await fs.readFile(filePath, "utf-8") - // When reading with "utf-8" encoding, content should be a string - return content.trim() - } catch (err) { - const errorCode = (err as NodeJS.ErrnoException).code - if (!errorCode || !["ENOENT", "EISDIR"].includes(errorCode)) { - throw err - } - return "" - } -} - /** * Get the path to a system prompt file for a specific mode */ diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 31ccb383c6..260ad2704f 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -50,7 +50,7 @@ import { McpServerManager } from "../../services/mcp/McpServerManager" import { ShadowCheckpointService } from "../../services/checkpoints/ShadowCheckpointService" import { CodeIndexManager } from "../../services/code-index/manager" import type { IndexProgressUpdate } from "../../services/code-index/interfaces/manager" -import { fileExistsAtPath } from "../../utils/fs" +import { fileExistsAtPath, safeReadFile } from "../../utils/fs" import { setTtsEnabled, setTtsSpeed } from "../../utils/tts" import { ContextProxy } from "../config/ContextProxy" import { ProviderSettingsManager } from "../config/ProviderSettingsManager" @@ -988,6 +988,16 @@ export class ClineProvider await this.initClineWithHistoryItem({ ...historyItem, rootTask, parentTask }) } + // Settings Directory + + async ensureSettingsDirectoryExists(): Promise { + const { getSettingsDirectoryPath } = await import("../../utils/storage") + const globalStoragePath = this.contextProxy.globalStorageUri.fsPath + return getSettingsDirectoryPath(globalStoragePath) + } + + // Custom Instructions + async updateCustomInstructions(instructions?: string) { const settingsDirPath = await this.ensureSettingsDirectoryExists() const customInstructionsFilePath = path.join(settingsDirPath, GlobalFileNames.customInstructions) @@ -1017,17 +1027,14 @@ export class ClineProvider } async refreshCustomInstructions(): Promise { + const content = await this.readCustomInstructionsFromFile() + await this.updateCustomInstructions(content) + } + + private async readCustomInstructionsFromFile(): Promise { const settingsDirPath = await this.ensureSettingsDirectoryExists() const customInstructionsFilePath = path.join(settingsDirPath, GlobalFileNames.customInstructions) - let content: string | undefined = undefined - try { - content = await fs.readFile(customInstructionsFilePath, "utf-8") - } catch (error) { - if ((error as NodeJS.ErrnoException).code !== "ENOENT") { - throw error - } - } - await this.updateCustomInstructions(content) + return await safeReadFile(customInstructionsFilePath) } // MCP @@ -1055,27 +1062,6 @@ export class ClineProvider return mcpServersDir } - async ensureSettingsDirectoryExists(): Promise { - const { getSettingsDirectoryPath } = await import("../../utils/storage") - const globalStoragePath = this.contextProxy.globalStorageUri.fsPath - return getSettingsDirectoryPath(globalStoragePath) - } - - private async readCustomInstructionsFromFile(): Promise { - const settingsDirPath = await this.ensureSettingsDirectoryExists() - const customInstructionsFilePath = path.join(settingsDirPath, GlobalFileNames.customInstructions) - - if (await fileExistsAtPath(customInstructionsFilePath)) { - try { - return await fs.readFile(customInstructionsFilePath, "utf-8") - } catch (error) { - this.log(`Error reading custom instructions file: ${error}`) - return undefined - } - } - return undefined - } - // OpenRouter async handleOpenRouterCallback(code: string) { diff --git a/src/utils/fs.ts b/src/utils/fs.ts index 9f7af84e4a..1cd78f6ecf 100644 --- a/src/utils/fs.ts +++ b/src/utils/fs.ts @@ -45,3 +45,19 @@ export async function fileExistsAtPath(filePath: string): Promise { return false } } + +/** + * Safely read a file and return its trimmed content + */ +export async function safeReadFile(filePath: string): Promise { + try { + const content = await fs.readFile(filePath, "utf-8") + return content.trim() + } catch (err) { + const errorCode = (err as NodeJS.ErrnoException).code + if (!errorCode || !["ENOENT", "EISDIR"].includes(errorCode)) { + throw err + } + return "" + } +} From 86d8d05bdf46790d5fe3467d1a03e0d980bcc282 Mon Sep 17 00:00:00 2001 From: devxpain <170700110+devxpain@users.noreply.github.com> Date: Fri, 30 May 2025 21:15:32 +0800 Subject: [PATCH 3/7] fix(migration): handle migration error for custom instructions - Wrap file write and global state update in try-catch block - Log error message if migration fails --- src/utils/migrateSettings.ts | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/src/utils/migrateSettings.ts b/src/utils/migrateSettings.ts index b02daf89b3..45529b65f8 100644 --- a/src/utils/migrateSettings.ts +++ b/src/utils/migrateSettings.ts @@ -40,9 +40,15 @@ export async function migrateSettings( const customInstructionsFilePath = path.join(settingsDir, GlobalFileNames.customInstructions) if (customInstructionsContent && !(await fileExistsAtPath(customInstructionsFilePath))) { - await fs.writeFile(customInstructionsFilePath, customInstructionsContent, "utf-8") - await context.globalState.update(customInstructionsKey, undefined) // Delete from GlobalState - outputChannel.appendLine("Migrated custom instructions from GlobalState to file.") + try { + await fs.writeFile(customInstructionsFilePath, customInstructionsContent, "utf-8") + await context.globalState.update(customInstructionsKey, undefined) // Delete from GlobalState + outputChannel.appendLine("Migrated custom instructions from GlobalState to file.") + } catch (migrationError) { + outputChannel.appendLine( + `Error migrating custom instructions: ${migrationError}. Data might still be in GlobalState.`, + ) + } } else { outputChannel.appendLine( `Skipping custom instructions migration: ${customInstructionsContent ? "file already exists" : "no data in GlobalState"}`, From 1d8e86a7663748a11f5180356212ecca24e0fb5a Mon Sep 17 00:00:00 2001 From: devxpain <170700110+devxpain@users.noreply.github.com> Date: Sat, 31 May 2025 14:41:41 +0800 Subject: [PATCH 4/7] feat(content): generalize custom instructions and enhance refresh UX - Refactor custom instructions management to a generic 'content' system, introducing `GlobalContentIds`. - Implement new `openContent`, `updateContent`, and `refreshContent` methods in `ClineProvider` to handle various content types. - Update webview message interfaces (`WebviewMessage`, `ExtensionMessage`) to support content IDs and a new `contentRefreshed` event. - Enhance the custom instructions UI in the webview with a dedicated `useRefreshableContent` hook. - Add visual feedback for content refresh operations, including a loading spinner and a success message. --- src/core/webview/ClineProvider.ts | 85 +++++++++++++------ src/core/webview/webviewMessageHandler.ts | 22 +++-- src/shared/ExtensionMessage.ts | 2 + src/shared/WebviewMessage.ts | 5 +- src/shared/globalContentIds.ts | 3 + webview-ui/src/components/modes/ModesView.tsx | 29 ++++--- webview-ui/src/hooks/useRefreshableContent.ts | 45 ++++++++++ webview-ui/src/i18n/locales/en/prompts.json | 1 + 8 files changed, 147 insertions(+), 45 deletions(-) create mode 100644 src/shared/globalContentIds.ts create mode 100644 webview-ui/src/hooks/useRefreshableContent.ts diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 260ad2704f..4d0e257f9d 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -37,6 +37,7 @@ import { Package } from "../../shared/package" import { findLast } from "../../shared/array" import { supportPrompt } from "../../shared/support-prompt" import { GlobalFileNames } from "../../shared/globalFileNames" +import { GlobalContentIds } from "../../shared/globalContentIds" import { ExtensionMessage } from "../../shared/ExtensionMessage" import { Mode, defaultModeSlug } from "../../shared/modes" import { experimentDefault, experiments, EXPERIMENT_IDS } from "../../shared/experiments" @@ -996,45 +997,75 @@ export class ClineProvider return getSettingsDirectoryPath(globalStoragePath) } - // Custom Instructions + private async getContentPath(contentId: string): Promise { + const settingsDir = await this.ensureSettingsDirectoryExists() + switch (contentId) { + case GlobalContentIds.customInstructions: + return path.join(settingsDir, GlobalFileNames.customInstructions) + default: + this.log(`Unknown contentId: ${contentId}`) + return undefined + } + } + + // Content + + async openContent(contentId: string): Promise { + const filePath = await this.getContentPath(contentId) + + if (!filePath) { + this.log(`File path could not be determined for contentId: ${contentId}`) + return + } - async updateCustomInstructions(instructions?: string) { - const settingsDirPath = await this.ensureSettingsDirectoryExists() - const customInstructionsFilePath = path.join(settingsDirPath, GlobalFileNames.customInstructions) + const uri = vscode.Uri.file(filePath) + await vscode.commands.executeCommand("vscode.open", uri, { + preview: false, + preserveFocus: true, + }) + await vscode.commands.executeCommand("workbench.action.files.revert") + } - if (instructions && instructions.trim()) { - await fs.writeFile(customInstructionsFilePath, instructions.trim(), "utf-8") + async refreshContent(contentId: string): Promise { + const content = await this.readContent(contentId) + await this.updateContent(contentId, content) + this.postMessageToWebview({ + type: "contentRefreshed", + contentId: contentId, + success: true, // Assuming success for now + }) + } + + async updateContent(contentId: string, content?: string) { + const filePath = await this.getContentPath(contentId) + + if (!filePath) { + this.log(`File path could not be determined for contentId: ${contentId}`) + return + } + + if (content && content.trim()) { + await fs.writeFile(filePath, content.trim(), "utf-8") + this.log(`Updated content file: ${filePath}`) } else { - // If instructions are empty or undefined, delete the file if it exists try { - await fs.unlink(customInstructionsFilePath) + await fs.unlink(filePath) + this.log(`Deleted content file: ${filePath}`) } catch (error) { - // Ignore if file doesn't exist - if ((error as NodeJS.ErrnoException).code !== "ENOENT") { + if (error.code !== "ENOENT") { + this.log(`Error deleting content file: ${error.message}`) throw error } } } + // Update the webview state await this.postStateToWebview() } - async openCustomInstructionsFile(): Promise { - const settingsDirPath = await this.ensureSettingsDirectoryExists() - const customInstructionsFilePath = path.join(settingsDirPath, GlobalFileNames.customInstructions) - const fileUri = vscode.Uri.file(customInstructionsFilePath) - await vscode.commands.executeCommand("vscode.open", fileUri, { preview: false, preserveFocus: true }) - await vscode.commands.executeCommand("workbench.action.files.revert", fileUri) - } - - async refreshCustomInstructions(): Promise { - const content = await this.readCustomInstructionsFromFile() - await this.updateCustomInstructions(content) - } + private async readContent(contentId: string): Promise { + const filePath = await this.getContentPath(contentId) - private async readCustomInstructionsFromFile(): Promise { - const settingsDirPath = await this.ensureSettingsDirectoryExists() - const customInstructionsFilePath = path.join(settingsDirPath, GlobalFileNames.customInstructions) - return await safeReadFile(customInstructionsFilePath) + return filePath ? ((await safeReadFile(filePath)) ?? "") : "" } // MCP @@ -1511,7 +1542,7 @@ export class ClineProvider return { apiConfiguration: providerSettings, lastShownAnnouncementId: stateValues.lastShownAnnouncementId, - customInstructions: await this.readCustomInstructionsFromFile(), + customInstructions: await this.readContent(GlobalContentIds.customInstructions), apiModelId: stateValues.apiModelId, alwaysAllowReadOnly: stateValues.alwaysAllowReadOnly ?? false, alwaysAllowReadOnlyOutsideWorkspace: stateValues.alwaysAllowReadOnlyOutsideWorkspace ?? false, diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index 66dae30eef..938af1f913 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -14,9 +14,9 @@ import { RouterName, toRouterName, ModelRecord } from "../../shared/api" import { supportPrompt } from "../../shared/support-prompt" import { checkoutDiffPayloadSchema, checkoutRestorePayloadSchema, WebviewMessage } from "../../shared/WebviewMessage" +import { GlobalContentIds } from "../../shared/globalContentIds" import { checkExistKey } from "../../shared/checkExistApiConfig" import { experimentDefault } from "../../shared/experiments" -import { GlobalFileNames } from "../../shared/globalFileNames" import { Terminal } from "../../integrations/terminal/Terminal" import { openFile, openImage } from "../../integrations/misc/open-file" import { selectImages } from "../../integrations/misc/process-images" @@ -131,13 +131,23 @@ export const webviewMessageHandler = async (provider: ClineProvider, message: We await provider.initClineWithTask(message.text, message.images) break case "customInstructions": - await provider.updateCustomInstructions(message.text) + await provider.updateContent(GlobalContentIds.customInstructions, message.text) break - case "openCustomInstructionsFile": - await provider.openCustomInstructionsFile() + case "openContent": + if (message.contentId) { + // Check for contentId instead of filePath + await provider.openContent(message.contentId) + } else { + console.error("openContent message missing contentId") + } break - case "refreshCustomInstructions": - await provider.refreshCustomInstructions() + case "refreshContent": + if (message.contentId) { + await provider.refreshContent(message.contentId) + } else { + // Handle error or log if contentId is missing + console.error("refreshContent message missing contentId") + } break case "alwaysAllowReadOnly": await updateGlobalState("alwaysAllowReadOnly", message.bool ?? undefined) diff --git a/src/shared/ExtensionMessage.ts b/src/shared/ExtensionMessage.ts index 07d300ed01..92bb610ca8 100644 --- a/src/shared/ExtensionMessage.ts +++ b/src/shared/ExtensionMessage.ts @@ -73,6 +73,7 @@ export interface ExtensionMessage { | "indexingStatusUpdate" | "indexCleared" | "codebaseIndexConfig" + | "contentRefreshed" text?: string action?: | "chatButtonClicked" @@ -105,6 +106,7 @@ export interface ExtensionMessage { customMode?: ModeConfig slug?: string success?: boolean + contentId?: string values?: Record requestId?: string promptText?: string diff --git a/src/shared/WebviewMessage.ts b/src/shared/WebviewMessage.ts index 9fd637e7a8..66ec21d0ec 100644 --- a/src/shared/WebviewMessage.ts +++ b/src/shared/WebviewMessage.ts @@ -51,6 +51,7 @@ export interface WebviewMessage { | "openImage" | "openFile" | "openMention" + | "openContent" | "cancelTask" | "updateVSCodeSetting" | "getVSCodeSetting" @@ -118,8 +119,7 @@ export interface WebviewMessage { | "deleteCustomMode" | "setopenAiCustomModelInfo" | "openCustomModesSettings" - | "openCustomInstructionsFile" - | "refreshCustomInstructions" + | "refreshContent" | "checkpointDiff" | "checkpointRestore" | "deleteMcpServer" @@ -172,6 +172,7 @@ export interface WebviewMessage { modeConfig?: ModeConfig timeout?: number payload?: WebViewMessagePayload + contentId?: string source?: "global" | "project" requestId?: string ids?: string[] diff --git a/src/shared/globalContentIds.ts b/src/shared/globalContentIds.ts new file mode 100644 index 0000000000..c7c43ead5f --- /dev/null +++ b/src/shared/globalContentIds.ts @@ -0,0 +1,3 @@ +export const GlobalContentIds = { + customInstructions: "customInstructions" as const, +} diff --git a/webview-ui/src/components/modes/ModesView.tsx b/webview-ui/src/components/modes/ModesView.tsx index a423b02fc2..85b95d59a8 100644 --- a/webview-ui/src/components/modes/ModesView.tsx +++ b/webview-ui/src/components/modes/ModesView.tsx @@ -44,6 +44,8 @@ import { CommandGroup, Input, } from "@src/components/ui" +import { GlobalContentIds } from "../../../../src/shared/globalContentIds" +import { useRefreshableContent } from "../../hooks/useRefreshableContent" // Get all available groups that should show in prompts view const availableGroups = (Object.keys(TOOL_GROUPS) as ToolGroup[]).filter((group) => !TOOL_GROUPS[group].alwaysAvailable) @@ -79,6 +81,10 @@ const ModesView = ({ onDone }: ModesViewProps) => { // 3. Still sending the mode change to the backend for persistence const [visualMode, setVisualMode] = useState(mode) + const { isRefreshing, showRefreshSuccess, refreshContent } = useRefreshableContent( + GlobalContentIds.customInstructions, + ) + // Memoize modes to preserve array order const modes = useMemo(() => getAllModes(customModes), [customModes]) @@ -1045,7 +1051,8 @@ const ModesView = ({ onDone }: ModesViewProps) => { size="icon" onClick={() => { vscode.postMessage({ - type: "openCustomInstructionsFile", + type: "openContent", + contentId: GlobalContentIds.customInstructions, }) }} title={t("prompts:globalCustomInstructions.openFile")}> @@ -1054,17 +1061,19 @@ const ModesView = ({ onDone }: ModesViewProps) => {
- + {showRefreshSuccess && ( +
+ {t("prompts:globalCustomInstructions.refreshSuccess")} +
+ )}
{ ((e as any).target as HTMLTextAreaElement).value setCustomInstructions(value || undefined) vscode.postMessage({ - type: "customInstructions", + type: GlobalContentIds.customInstructions, text: value.trim() || undefined, }) }} diff --git a/webview-ui/src/hooks/useRefreshableContent.ts b/webview-ui/src/hooks/useRefreshableContent.ts new file mode 100644 index 0000000000..e48063fe6b --- /dev/null +++ b/webview-ui/src/hooks/useRefreshableContent.ts @@ -0,0 +1,45 @@ +import { useState, useEffect, useCallback } from "react" +import { vscode } from "../utils/vscode" +import type { ExtensionMessage } from "../../../src/shared/ExtensionMessage" + +interface UseRefreshableContentResult { + isRefreshing: boolean + showRefreshSuccess: boolean + refreshContent: () => void +} + +export function useRefreshableContent(contentId: string): UseRefreshableContentResult { + const [isRefreshing, setIsRefreshing] = useState(false) + const [showRefreshSuccess, setShowRefreshSuccess] = useState(false) + + useEffect(() => { + const handleMessage = (event: MessageEvent) => { + const message = event.data + if (message.type === "contentRefreshed" && message.contentId === contentId) { + setIsRefreshing(false) + if (message.success) { + setShowRefreshSuccess(true) + const timer = setTimeout(() => { + setShowRefreshSuccess(false) + }, 3000) + return () => clearTimeout(timer) + } + } + } + + window.addEventListener("message", handleMessage) + return () => { + window.removeEventListener("message", handleMessage) + } + }, [contentId]) + + const refreshContent = useCallback(() => { + setIsRefreshing(true) + vscode.postMessage({ + type: "refreshContent", + contentId, + }) + }, [contentId]) + + return { isRefreshing, showRefreshSuccess, refreshContent } +} diff --git a/webview-ui/src/i18n/locales/en/prompts.json b/webview-ui/src/i18n/locales/en/prompts.json index 20f534f56a..531bdfec62 100644 --- a/webview-ui/src/i18n/locales/en/prompts.json +++ b/webview-ui/src/i18n/locales/en/prompts.json @@ -50,6 +50,7 @@ "description": "These instructions apply to all modes. They provide a base set of behaviors that can be enhanced by mode-specific instructions below. <0>Learn more", "openFile": "Open custom instructions file", "refreshFile": "Refresh custom instructions from file", + "refreshSuccess": "Custom instructions refreshed!", "loadFromFile": "Instructions can also be loaded from the .roo/rules/ folder in your workspace (.roorules and .clinerules are deprecated and will stop working soon)." }, "systemPrompt": { From bf6cc42ded6a77301a53933a76e53f253ae3aa2e Mon Sep 17 00:00:00 2001 From: devxpain <170700110+devxpain@users.noreply.github.com> Date: Sat, 31 May 2025 16:49:19 +0800 Subject: [PATCH 5/7] refactor(webview): use `openFile` utility for file opening - Replaced direct `vscode.commands.executeCommand("vscode.open")` call with the `openFile` utility. - Centralizes file opening logic, improving reusability and maintainability within the webview provider. --- src/core/webview/ClineProvider.ts | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 4d0e257f9d..3738797b79 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -52,6 +52,7 @@ import { ShadowCheckpointService } from "../../services/checkpoints/ShadowCheckp import { CodeIndexManager } from "../../services/code-index/manager" import type { IndexProgressUpdate } from "../../services/code-index/interfaces/manager" import { fileExistsAtPath, safeReadFile } from "../../utils/fs" +import { openFile } from "../../integrations/misc/open-file" import { setTtsEnabled, setTtsSpeed } from "../../utils/tts" import { ContextProxy } from "../config/ContextProxy" import { ProviderSettingsManager } from "../config/ProviderSettingsManager" @@ -1018,11 +1019,7 @@ export class ClineProvider return } - const uri = vscode.Uri.file(filePath) - await vscode.commands.executeCommand("vscode.open", uri, { - preview: false, - preserveFocus: true, - }) + await openFile(filePath, { create: true }) await vscode.commands.executeCommand("workbench.action.files.revert") } From 4f233b99d648650c502a189733694109c60c9aee Mon Sep 17 00:00:00 2001 From: devxpain <170700110+devxpain@users.noreply.github.com> Date: Mon, 2 Jun 2025 14:27:06 +0800 Subject: [PATCH 6/7] refactor(test): improve test mocks and fs utility - Ensure `safeReadFile` handles empty content gracefully to prevent potential errors. - Update Jest mocks for `fs` utility functions in tests to correctly spread original exports, improving test reliability. - Add comprehensive `globalState` mocking in `migrateSettings` tests for better test coverage. --- src/__tests__/migrateSettings.test.ts | 12 ++++++++++++ .../prompts/__tests__/custom-system-prompt.test.ts | 1 + src/core/task/__tests__/Task.test.ts | 1 + src/utils/fs.ts | 2 +- 4 files changed, 15 insertions(+), 1 deletion(-) diff --git a/src/__tests__/migrateSettings.test.ts b/src/__tests__/migrateSettings.test.ts index 538ff7d590..35640c885f 100644 --- a/src/__tests__/migrateSettings.test.ts +++ b/src/__tests__/migrateSettings.test.ts @@ -48,6 +48,18 @@ describe("Settings Migration", () => { // Mock extension context mockContext = { globalStorageUri: { fsPath: mockStoragePath }, + globalState: { + get: jest.fn((key: string) => { + if (key === "clineCustomModes") { + return "some old custom modes content" + } + if (key === "customInstructions") { + return undefined + } + return undefined + }), + update: jest.fn().mockResolvedValue(undefined), + }, } as unknown as vscode.ExtensionContext // Set global outputChannel for all tests diff --git a/src/core/prompts/__tests__/custom-system-prompt.test.ts b/src/core/prompts/__tests__/custom-system-prompt.test.ts index e7d1ae08d7..7d11f992da 100644 --- a/src/core/prompts/__tests__/custom-system-prompt.test.ts +++ b/src/core/prompts/__tests__/custom-system-prompt.test.ts @@ -16,6 +16,7 @@ const mockedFs = fs as jest.Mocked // Mock the fileExistsAtPath function jest.mock("../../../utils/fs", () => ({ + ...(jest.requireActual("../../../utils/fs") as object), // Spread all original exports fileExistsAtPath: jest.fn().mockResolvedValue(true), createDirectoriesForFile: jest.fn().mockResolvedValue([]), })) diff --git a/src/core/task/__tests__/Task.test.ts b/src/core/task/__tests__/Task.test.ts index 8ed57ffcb3..78f500327b 100644 --- a/src/core/task/__tests__/Task.test.ts +++ b/src/core/task/__tests__/Task.test.ts @@ -140,6 +140,7 @@ jest.mock("../../../utils/storage", () => ({ })) jest.mock("../../../utils/fs", () => ({ + ...(jest.requireActual("../../../utils/fs") as object), // Spread all original exports fileExistsAtPath: jest.fn().mockImplementation((filePath) => { return filePath.includes("ui_messages.json") || filePath.includes("api_conversation_history.json") }), diff --git a/src/utils/fs.ts b/src/utils/fs.ts index 1cd78f6ecf..6bb0ed7fb6 100644 --- a/src/utils/fs.ts +++ b/src/utils/fs.ts @@ -52,7 +52,7 @@ export async function fileExistsAtPath(filePath: string): Promise { export async function safeReadFile(filePath: string): Promise { try { const content = await fs.readFile(filePath, "utf-8") - return content.trim() + return content ? content.trim() : "" } catch (err) { const errorCode = (err as NodeJS.ErrnoException).code if (!errorCode || !["ENOENT", "EISDIR"].includes(errorCode)) { From d5f522fa82d8e434729413f1d071015011865760 Mon Sep 17 00:00:00 2001 From: devxpain <170700110+devxpain@users.noreply.github.com> Date: Sat, 7 Jun 2025 22:51:06 +0800 Subject: [PATCH 7/7] refactor(content): extract content management to dedicated service - Move content-related methods (openContent, refreshContent, updateContent, readContent) from ClineProvider to a new ContentManager class. - Instantiate ContentManager in ClineProvider and delegate content operations to it. - Improve separation of concerns by centralizing content file system interactions and state updates within ContentManager. --- src/core/webview/ClineProvider.ts | 71 +++------------ src/core/webview/webviewMessageHandler.ts | 6 +- src/services/content/ContentManager.ts | 102 ++++++++++++++++++++++ 3 files changed, 117 insertions(+), 62 deletions(-) create mode 100644 src/services/content/ContentManager.ts diff --git a/src/core/webview/ClineProvider.ts b/src/core/webview/ClineProvider.ts index 3738797b79..ff0cdd5b14 100644 --- a/src/core/webview/ClineProvider.ts +++ b/src/core/webview/ClineProvider.ts @@ -51,8 +51,7 @@ import { McpServerManager } from "../../services/mcp/McpServerManager" import { ShadowCheckpointService } from "../../services/checkpoints/ShadowCheckpointService" import { CodeIndexManager } from "../../services/code-index/manager" import type { IndexProgressUpdate } from "../../services/code-index/interfaces/manager" -import { fileExistsAtPath, safeReadFile } from "../../utils/fs" -import { openFile } from "../../integrations/misc/open-file" +import { fileExistsAtPath } from "../../utils/fs" import { setTtsEnabled, setTtsSpeed } from "../../utils/tts" import { ContextProxy } from "../config/ContextProxy" import { ProviderSettingsManager } from "../config/ProviderSettingsManager" @@ -67,6 +66,7 @@ import { webviewMessageHandler } from "./webviewMessageHandler" import { WebviewMessage } from "../../shared/WebviewMessage" import { EMBEDDING_MODEL_PROFILES } from "../../shared/embeddingModels" import { ProfileValidator } from "../../shared/ProfileValidator" +import { ContentManager } from "../../services/content/ContentManager" /** * https://github.com/microsoft/vscode-webview-ui-toolkit-samples/blob/main/default/weather-webview/src/providers/WeatherViewProvider.ts @@ -103,6 +103,7 @@ export class ClineProvider return this._workspaceTracker } protected mcpHub?: McpHub // Change from private to protected + public contentManager: ContentManager // Declare private member public isViewLaunched = false public settingsImportedAt?: number @@ -149,6 +150,14 @@ export class ClineProvider .catch((error) => { this.log(`Failed to initialize MCP Hub: ${error}`) }) + + // Instantiate ContentManager + this.contentManager = new ContentManager({ + log: this.log.bind(this), + postMessageToWebview: this.postMessageToWebview.bind(this), + postStateToWebview: this.postStateToWebview.bind(this), + globalStorageUriFsPath: this.contextProxy.globalStorageUri.fsPath, + }) } // Adds a new Cline instance to clineStack, marking the start of a new task. @@ -1009,62 +1018,6 @@ export class ClineProvider } } - // Content - - async openContent(contentId: string): Promise { - const filePath = await this.getContentPath(contentId) - - if (!filePath) { - this.log(`File path could not be determined for contentId: ${contentId}`) - return - } - - await openFile(filePath, { create: true }) - await vscode.commands.executeCommand("workbench.action.files.revert") - } - - async refreshContent(contentId: string): Promise { - const content = await this.readContent(contentId) - await this.updateContent(contentId, content) - this.postMessageToWebview({ - type: "contentRefreshed", - contentId: contentId, - success: true, // Assuming success for now - }) - } - - async updateContent(contentId: string, content?: string) { - const filePath = await this.getContentPath(contentId) - - if (!filePath) { - this.log(`File path could not be determined for contentId: ${contentId}`) - return - } - - if (content && content.trim()) { - await fs.writeFile(filePath, content.trim(), "utf-8") - this.log(`Updated content file: ${filePath}`) - } else { - try { - await fs.unlink(filePath) - this.log(`Deleted content file: ${filePath}`) - } catch (error) { - if (error.code !== "ENOENT") { - this.log(`Error deleting content file: ${error.message}`) - throw error - } - } - } - // Update the webview state - await this.postStateToWebview() - } - - private async readContent(contentId: string): Promise { - const filePath = await this.getContentPath(contentId) - - return filePath ? ((await safeReadFile(filePath)) ?? "") : "" - } - // MCP async ensureMcpServersDirectoryExists(): Promise { @@ -1539,7 +1492,7 @@ export class ClineProvider return { apiConfiguration: providerSettings, lastShownAnnouncementId: stateValues.lastShownAnnouncementId, - customInstructions: await this.readContent(GlobalContentIds.customInstructions), + customInstructions: await this.contentManager.readContent(GlobalContentIds.customInstructions), apiModelId: stateValues.apiModelId, alwaysAllowReadOnly: stateValues.alwaysAllowReadOnly ?? false, alwaysAllowReadOnlyOutsideWorkspace: stateValues.alwaysAllowReadOnlyOutsideWorkspace ?? false, diff --git a/src/core/webview/webviewMessageHandler.ts b/src/core/webview/webviewMessageHandler.ts index 938af1f913..60139e679b 100644 --- a/src/core/webview/webviewMessageHandler.ts +++ b/src/core/webview/webviewMessageHandler.ts @@ -131,19 +131,19 @@ export const webviewMessageHandler = async (provider: ClineProvider, message: We await provider.initClineWithTask(message.text, message.images) break case "customInstructions": - await provider.updateContent(GlobalContentIds.customInstructions, message.text) + await provider.contentManager.updateContent(GlobalContentIds.customInstructions, message.text) break case "openContent": if (message.contentId) { // Check for contentId instead of filePath - await provider.openContent(message.contentId) + await provider.contentManager.openContent(message.contentId) } else { console.error("openContent message missing contentId") } break case "refreshContent": if (message.contentId) { - await provider.refreshContent(message.contentId) + await provider.contentManager.refreshContent(message.contentId) } else { // Handle error or log if contentId is missing console.error("refreshContent message missing contentId") diff --git a/src/services/content/ContentManager.ts b/src/services/content/ContentManager.ts new file mode 100644 index 0000000000..631fb4f021 --- /dev/null +++ b/src/services/content/ContentManager.ts @@ -0,0 +1,102 @@ +import * as vscode from "vscode" +import * as path from "path" +import fs from "fs/promises" + +import { GlobalContentIds } from "../../shared/globalContentIds" +import { GlobalFileNames } from "../../shared/globalFileNames" +import { openFile } from "../../integrations/misc/open-file" +import { safeReadFile } from "../../utils/fs" +import { getSettingsDirectoryPath } from "../../utils/storage" // Need to confirm this import path +import { ExtensionMessage } from "../../shared/ExtensionMessage" // Assuming this type is needed for postMessageToWebview + +// Define an interface for the dependencies ContentManager needs from ClineProvider +interface ContentManagerDependencies { + log: (message: string) => void + postMessageToWebview: (message: ExtensionMessage) => Promise + postStateToWebview: () => Promise + globalStorageUriFsPath: string // To replace contextProxy.globalStorageUri.fsPath +} + +export class ContentManager { + private readonly log: (message: string) => void + private readonly postMessageToWebview: (message: ExtensionMessage) => Promise + private readonly postStateToWebview: () => Promise + private readonly globalStorageUriFsPath: string + + constructor(dependencies: ContentManagerDependencies) { + this.log = dependencies.log + this.postMessageToWebview = dependencies.postMessageToWebview + this.postStateToWebview = dependencies.postStateToWebview + this.globalStorageUriFsPath = dependencies.globalStorageUriFsPath + } + + private async ensureSettingsDirectoryExists(): Promise { + const globalStoragePath = this.globalStorageUriFsPath + return getSettingsDirectoryPath(globalStoragePath) + } + + private async getContentPath(contentId: string): Promise { + const settingsDir = await this.ensureSettingsDirectoryExists() + switch (contentId) { + case GlobalContentIds.customInstructions: + return path.join(settingsDir, GlobalFileNames.customInstructions) + default: + this.log(`Unknown contentId: ${contentId}`) + return undefined + } + } + + async openContent(contentId: string): Promise { + const filePath = await this.getContentPath(contentId) + + if (!filePath) { + this.log(`File path could not be determined for contentId: ${contentId}`) + return + } + + await openFile(filePath, { create: true }) + await vscode.commands.executeCommand("workbench.action.files.revert") + } + + async refreshContent(contentId: string): Promise { + const content = await this.readContent(contentId) + await this.updateContent(contentId, content) + this.postMessageToWebview({ + type: "contentRefreshed", + contentId: contentId, + success: true, // Assuming success for now + }) + } + + async updateContent(contentId: string, content?: string) { + const filePath = await this.getContentPath(contentId) + + if (!filePath) { + this.log(`File path could not be determined for contentId: ${contentId}`) + return + } + + if (content && content.trim()) { + await fs.writeFile(filePath, content.trim(), "utf-8") + this.log(`Updated content file: ${filePath}`) + } else { + try { + await fs.unlink(filePath) + this.log(`Deleted content file: ${filePath}`) + } catch (error: any) { + if (error.code !== "ENOENT") { + this.log(`Error deleting content file: ${error.message}`) + throw error + } + } + } + // Update the webview state + await this.postStateToWebview() + } + + async readContent(contentId: string): Promise { + const filePath = await this.getContentPath(contentId) + + return filePath ? ((await safeReadFile(filePath)) ?? "") : "" + } +}