From 63b77a62132602f143a685085ea29f0e33e9cab2 Mon Sep 17 00:00:00 2001 From: "Duong M. CUONG" Date: Tue, 8 Apr 2025 04:24:30 +0900 Subject: [PATCH 1/2] feat: Add support for custom instructions paths in custom mode config - Implemented functionality in `custom-instruction.ts` to handle loading rules from custom instruction paths. - Refactored and fixed previously incorrect logic in `custom-instruction.ts` for better reliability and maintainability. (comment in #2354) - Enhanced `system.ts` to pass custom instruction paths to `addCustomInstructions`. - Extended `roo-code.d.ts` and `types.ts` to include `customInstructionsPaths` in `ModeConfig`. - Modified `index.ts` schema to define `customInstructionsPathsConfigSchema` and integrate it into `ModeConfig`. - Updated `modes.ts` to support `customInstructionsPaths` in mode configuration and full mode details. --- .../__tests__/custom-instructions.test.ts | 23 +++--- .../prompts/sections/custom-instructions.ts | 72 +++++++++++++++---- src/core/prompts/system.ts | 8 ++- src/exports/roo-code.d.ts | 9 +++ src/exports/types.ts | 9 +++ src/schemas/index.ts | 10 +++ src/shared/modes.ts | 2 +- 7 files changed, 103 insertions(+), 30 deletions(-) diff --git a/src/core/prompts/sections/__tests__/custom-instructions.test.ts b/src/core/prompts/sections/__tests__/custom-instructions.test.ts index 1871b4995e5..ebb1c34757b 100644 --- a/src/core/prompts/sections/__tests__/custom-instructions.test.ts +++ b/src/core/prompts/sections/__tests__/custom-instructions.test.ts @@ -2,6 +2,7 @@ import { loadRuleFiles, addCustomInstructions } from "../custom-instructions" import fs from "fs/promises" import path from "path" import { PathLike } from "fs" +import { parentPort } from "worker_threads" // Mock fs/promises jest.mock("fs/promises") @@ -127,8 +128,8 @@ describe("loadRuleFiles", () => { // Simulate listing files readdirMock.mockResolvedValueOnce([ - { name: "file1.txt", isFile: () => true }, - { name: "file2.txt", isFile: () => true }, + { name: "file1.txt", isFile: () => true, parentPath: "/fake/path/.roo/rules" }, + { name: "file2.txt", isFile: () => true, parentPath: "/fake/path/.roo/rules" }, ] as any) statMock.mockImplementation( @@ -154,8 +155,6 @@ describe("loadRuleFiles", () => { expect(result).toContain("# Rules from /fake/path/.roo/rules/file2.txt:") expect(result).toContain("content of file2") - expect(statMock).toHaveBeenCalledWith("/fake/path/.roo/rules/file1.txt") - expect(statMock).toHaveBeenCalledWith("/fake/path/.roo/rules/file2.txt") expect(readFileMock).toHaveBeenCalledWith("/fake/path/.roo/rules/file1.txt", "utf-8") expect(readFileMock).toHaveBeenCalledWith("/fake/path/.roo/rules/file2.txt", "utf-8") }) @@ -321,8 +320,8 @@ describe("addCustomInstructions", () => { // Simulate listing files readdirMock.mockResolvedValueOnce([ - { name: "rule1.txt", isFile: () => true }, - { name: "rule2.txt", isFile: () => true }, + { name: "rule1.txt", isFile: () => true, parentPath: "/fake/path/.roo/rules-test-mode" }, + { name: "rule2.txt", isFile: () => true, parentPath: "/fake/path/.roo/rules-test-mode" }, ] as any) statMock.mockImplementation( @@ -356,8 +355,6 @@ describe("addCustomInstructions", () => { expect(result).toContain("# Rules from /fake/path/.roo/rules-test-mode/rule2.txt:") expect(result).toContain("mode specific rule 2") - expect(statMock).toHaveBeenCalledWith("/fake/path/.roo/rules-test-mode/rule1.txt") - expect(statMock).toHaveBeenCalledWith("/fake/path/.roo/rules-test-mode/rule2.txt") expect(readFileMock).toHaveBeenCalledWith("/fake/path/.roo/rules-test-mode/rule1.txt", "utf-8") expect(readFileMock).toHaveBeenCalledWith("/fake/path/.roo/rules-test-mode/rule2.txt", "utf-8") }) @@ -422,7 +419,9 @@ describe("addCustomInstructions", () => { ) // Simulate directory has files - readdirMock.mockResolvedValueOnce([{ name: "rule1.txt", isFile: () => true }] as any) + readdirMock.mockResolvedValueOnce([ + { name: "rule1.txt", isFile: () => true, parentPath: "/fake/path/.roo/rules-test-mode" }, + ] as any) readFileMock.mockReset() // Set up stat mock for checking files @@ -515,9 +514,9 @@ describe("Rules directory reading", () => { // Simulate listing files readdirMock.mockResolvedValueOnce([ - { name: "file1.txt", isFile: () => true }, - { name: "file2.txt", isFile: () => true }, - { name: "file3.txt", isFile: () => true }, + { name: "file1.txt", isFile: () => true, parentPath: "/fake/path/.roo/rules" }, + { name: "file2.txt", isFile: () => true, parentPath: "/fake/path/.roo/rules" }, + { name: "file3.txt", isFile: () => true, parentPath: "/fake/path/.roo/rules" }, ] as any) statMock.mockImplementation((path) => { diff --git a/src/core/prompts/sections/custom-instructions.ts b/src/core/prompts/sections/custom-instructions.ts index b17fc1f3194..48549f74f9c 100644 --- a/src/core/prompts/sections/custom-instructions.ts +++ b/src/core/prompts/sections/custom-instructions.ts @@ -2,6 +2,7 @@ import fs from "fs/promises" import path from "path" import { LANGUAGES, isLanguage } from "../../../shared/language" +import { CustomInstructionsPathsConfig } from "../../../schemas" /** * Safely read a file and return its trimmed content @@ -34,23 +35,18 @@ async function directoryExists(dirPath: string): Promise { /** * Read all text files from a directory in alphabetical order */ -async function readTextFilesFromDirectory(dirPath: string): Promise> { +async function readTextFilesFromDirectory(dirPath: string): Promise> { try { const files = await fs .readdir(dirPath, { withFileTypes: true, recursive: true }) .then((files) => files.filter((file) => file.isFile())) - .then((files) => files.map((file) => path.resolve(dirPath, file.name))) + .then((files) => files.map((file) => path.join(file.parentPath, file.name))) const fileContents = await Promise.all( files.map(async (file) => { try { - // Check if it's a file (not a directory) - const stats = await fs.stat(file) - if (stats.isFile()) { - const content = await safeReadFile(file) - return { filename: file, content } - } - return null + const content = await safeReadFile(file) + return { fullPath: file, content } } catch (err) { return null } @@ -58,7 +54,7 @@ async function readTextFilesFromDirectory(dirPath: string): Promise item !== null) + return fileContents.filter((item) => item !== null) } catch (err) { return [] } @@ -67,14 +63,14 @@ async function readTextFilesFromDirectory(dirPath: string): Promise): string { +function formatDirectoryContent(dirPath: string, files: Array<{ fullPath: string; content: string }>): string { if (files.length === 0) return "" return ( "\n\n" + files .map((file) => { - return `# Rules from ${file.filename}:\n${file.content}:` + return `# Rules from ${file.fullPath}:\n${file.content}:` }) .join("\n\n") ) @@ -106,12 +102,40 @@ export async function loadRuleFiles(cwd: string): Promise { return "" } +/** + * Load rules from a directory or file. + * If the path is a directory, it will read all files in that directory recursively. + * If the path is a file, it will read that file. + * @param fullPath - The full path to the directory or file. + * @returns An array of rules, each representing the content of a rule file. + */ +export async function loadRulesInPath(fullPath: string): Promise { + const rules: string[] = [] + const stats = await fs.lstat(fullPath) + if (stats.isDirectory()) { + const ruleFiles = await readTextFilesFromDirectory(fullPath) + if (ruleFiles.length > 0) { + rules.push(...ruleFiles.map((ruleFile) => `# Rules from ${ruleFile.fullPath}:\n${ruleFile.content}`)) + } + } else { + const content = await safeReadFile(fullPath) + if (content) { + rules.push(`# Rules from ${fullPath}:\n${content}`) + } + } + return rules +} + export async function addCustomInstructions( modeCustomInstructions: string, globalCustomInstructions: string, cwd: string, mode: string, - options: { language?: string; rooIgnoreInstructions?: string } = {}, + options: { + language?: string + rooIgnoreInstructions?: string + customInstructionsPaths?: CustomInstructionsPathsConfig[] + } = {}, ): Promise { const sections = [] @@ -176,14 +200,32 @@ export async function addCustomInstructions( } } + // Load custom instructions from paths if provided + if (options.customInstructionsPaths) { + const customInstructionsFullPaths = options.customInstructionsPaths.map((customPath) => { + return typeof customPath === "string" + ? path.isAbsolute(customPath) + ? customPath + : path.join(cwd, customPath) + : customPath.isAbsolute || path.isAbsolute(customPath.path) + ? customPath.path + : path.join(cwd, customPath.path) + }) + // TODO: Consider to use glob pattern + for (const customInstructionsFullPath of customInstructionsFullPaths) { + const loadedRules = await loadRulesInPath(customInstructionsFullPath) + rules.push(...loadedRules) + } + } + if (options.rooIgnoreInstructions) { rules.push(options.rooIgnoreInstructions) } // Add generic rules const genericRuleContent = await loadRuleFiles(cwd) - if (genericRuleContent && genericRuleContent.trim()) { - rules.push(genericRuleContent.trim()) + if (genericRuleContent) { + rules.push(genericRuleContent) } if (rules.length > 0) { diff --git a/src/core/prompts/system.ts b/src/core/prompts/system.ts index db06980175d..bcc3c6dc026 100644 --- a/src/core/prompts/system.ts +++ b/src/core/prompts/system.ts @@ -91,7 +91,7 @@ ${getSystemInfoSection(cwd, mode, customModeConfigs)} ${getObjectiveSection()} -${await addCustomInstructions(promptComponent?.customInstructions || modeConfig.customInstructions || "", globalCustomInstructions || "", cwd, mode, { language: language ?? formatLanguage(vscode.env.language), rooIgnoreInstructions })}` +${await addCustomInstructions(promptComponent?.customInstructions || modeConfig.customInstructions || "", globalCustomInstructions || "", cwd, mode, { language: language ?? formatLanguage(vscode.env.language), rooIgnoreInstructions, customInstructionsPaths: modeConfig.customInstructionsPaths })}` return basePrompt } @@ -141,7 +141,11 @@ export const SYSTEM_PROMPT = async ( globalCustomInstructions || "", cwd, mode, - { language: language ?? formatLanguage(vscode.env.language), rooIgnoreInstructions }, + { + language: language ?? formatLanguage(vscode.env.language), + rooIgnoreInstructions, + customInstructionsPaths: currentMode.customInstructionsPaths, + }, ) // For file-based prompts, don't include the tool sections return `${roleDefinition} diff --git a/src/exports/roo-code.d.ts b/src/exports/roo-code.d.ts index 087edc37c73..b7cf7088846 100644 --- a/src/exports/roo-code.d.ts +++ b/src/exports/roo-code.d.ts @@ -307,6 +307,15 @@ type GlobalSettings = { name: string roleDefinition: string customInstructions?: string | undefined + customInstructionsPaths?: + | ( + | { + path: string + isAbsolute?: boolean | undefined + } + | string + )[] + | undefined groups: ( | ("read" | "edit" | "browser" | "command" | "mcp" | "modes") | [ diff --git a/src/exports/types.ts b/src/exports/types.ts index 1cd4df7e57d..6f01bffde52 100644 --- a/src/exports/types.ts +++ b/src/exports/types.ts @@ -310,6 +310,15 @@ type GlobalSettings = { name: string roleDefinition: string customInstructions?: string | undefined + customInstructionsPaths?: + | ( + | { + path: string + isAbsolute?: boolean | undefined + } + | string + )[] + | undefined groups: ( | ("read" | "edit" | "browser" | "command" | "mcp" | "modes") | [ diff --git a/src/schemas/index.ts b/src/schemas/index.ts index bd45b99667e..fc3b35587a3 100644 --- a/src/schemas/index.ts +++ b/src/schemas/index.ts @@ -210,11 +210,21 @@ const groupEntryArraySchema = z.array(groupEntrySchema).refine( { message: "Duplicate groups are not allowed" }, ) +export const customInstructionsPathsConfigSchema = z.union([ + z.object({ + path: z.string(), + isAbsolute: z.boolean().optional(), + }), + z.string(), +]) +export type CustomInstructionsPathsConfig = z.infer + export const modeConfigSchema = z.object({ slug: z.string().regex(/^[a-zA-Z0-9-]+$/, "Slug must contain only letters numbers and dashes"), name: z.string().min(1, "Name is required"), roleDefinition: z.string().min(1, "Role definition is required"), customInstructions: z.string().optional(), + customInstructionsPaths: z.array(customInstructionsPathsConfigSchema).optional(), groups: groupEntryArraySchema, source: z.enum(["global", "project"]).optional(), }) diff --git a/src/shared/modes.ts b/src/shared/modes.ts index 0f51b4e50fe..7d6c96d66b7 100644 --- a/src/shared/modes.ts +++ b/src/shared/modes.ts @@ -272,7 +272,7 @@ export async function getFullModeDetails( options.globalCustomInstructions || "", options.cwd, modeSlug, - { language: options.language }, + { language: options.language, customInstructionsPaths: baseMode.customInstructionsPaths }, ) } From 9d4ad13b6a825a1caa407ab16eee1a91f56eacf2 Mon Sep 17 00:00:00 2001 From: "Duong M. CUONG" Date: Wed, 9 Apr 2025 04:34:56 +0900 Subject: [PATCH 2/2] resolve merge conflict and test script --- .../prompts/sections/__tests__/custom-instructions.test.ts | 5 ----- src/core/prompts/sections/custom-instructions.ts | 2 +- 2 files changed, 1 insertion(+), 6 deletions(-) diff --git a/src/core/prompts/sections/__tests__/custom-instructions.test.ts b/src/core/prompts/sections/__tests__/custom-instructions.test.ts index 92963e54f7b..9cc0d233152 100644 --- a/src/core/prompts/sections/__tests__/custom-instructions.test.ts +++ b/src/core/prompts/sections/__tests__/custom-instructions.test.ts @@ -264,11 +264,6 @@ describe("loadRuleFiles", () => { expect(result).toContain("# Rules from /fake/path/.roo/rules/subdir/subdir2/nested2.txt:") expect(result).toContain("nested file 2 content") - // Verify correct paths were checked - expect(statMock).toHaveBeenCalledWith("/fake/path/.roo/rules/root.txt") - expect(statMock).toHaveBeenCalledWith("/fake/path/.roo/rules/subdir/nested1.txt") - expect(statMock).toHaveBeenCalledWith("/fake/path/.roo/rules/subdir/subdir2/nested2.txt") - // Verify files were read with correct paths expect(readFileMock).toHaveBeenCalledWith("/fake/path/.roo/rules/root.txt", "utf-8") expect(readFileMock).toHaveBeenCalledWith("/fake/path/.roo/rules/subdir/nested1.txt", "utf-8") diff --git a/src/core/prompts/sections/custom-instructions.ts b/src/core/prompts/sections/custom-instructions.ts index 5eb2094bb4b..e3df1dcf750 100644 --- a/src/core/prompts/sections/custom-instructions.ts +++ b/src/core/prompts/sections/custom-instructions.ts @@ -70,7 +70,7 @@ function formatDirectoryContent(dirPath: string, files: Array<{ fullPath: string "\n\n" + files .map((file) => { - return `# Rules from ${file.filename}:\n${file.content}` + return `# Rules from ${file.fullPath}:\n${file.content}` }) .join("\n\n") )