-
Notifications
You must be signed in to change notification settings - Fork 2.6k
feat: add support for custom instruction file paths #7444
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
Conversation
- Add customInstructionPaths field to global settings schema - Implement loadCustomInstructionFiles function with support for: - Single file paths - Directory paths (loads all .md and .txt files) - Glob patterns for flexible file selection - Add security validation to prevent loading files outside workspace - Support parent directory access for monorepo scenarios - Integrate custom instructions into system prompt generation - Pass customInstructionPaths through state management - Add comprehensive unit tests for the new functionality Fixes #6168
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.
I reviewed my own code and found it surprisingly coherent. Must be a bug in the review process.
| // Security validation: ensure path is within workspace or parent directories | ||
| const normalizedPath = path.normalize(resolvedPath) | ||
| const normalizedCwd = path.normalize(cwd) | ||
| const parentDir = path.dirname(normalizedCwd) |
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.
Is this intentional? The parent directory access via path.dirname(normalizedCwd) could potentially be exploited in edge cases. Consider adding a configurable depth limit or explicit allowlist for parent directory access to strengthen security.
| import path from "path" | ||
| import * as os from "os" | ||
| import { Dirent } from "fs" | ||
| import { glob } from "glob" |
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.
Missing dependency alert: The glob package is imported here but I don't see it in package.json dependencies. This will cause runtime errors. Please add:
| import { glob } from "glob" | |
| // Add to package.json dependencies: | |
| "glob": "^10.3.10" |
| } | ||
| } | ||
| } catch (error) { | ||
| console.warn(`Error processing custom instruction path "${pathPattern}":`, error) |
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.
Could we improve error visibility here? When glob patterns fail, errors are only logged to console. Consider collecting these errors and returning them in a summary message so users know which paths failed to load.
| const MAX_FILE_SIZE = 1024 * 1024 // 1MB limit per file | ||
| const ALLOWED_EXTENSIONS = [".md", ".markdown", ".txt"] | ||
|
|
||
| for (const pathPattern of customPaths) { |
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.
Performance consideration: For large directories or complex glob patterns, this synchronous processing could be slow. Would it make sense to add a file count limit or process files in parallel batches?
| lastModeExportPath: z.string().optional(), | ||
| lastModeImportPath: z.string().optional(), | ||
|
|
||
| /** |
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.
The JSDoc is helpful, but could we add examples of valid path patterns? Something like:
| /** | |
| /** | |
| * Custom instruction file paths, directories, or glob patterns. | |
| * Supports single files, directories, glob patterns, and parent directory paths. | |
| * Examples: | |
| * - ".github/copilot-instructions.md" (single file) | |
| * - "docs/guidelines/" (directory) | |
| * - "**/*.instructions.md" (glob pattern) | |
| * - "../shared/instructions.md" (parent directory) | |
| */ |
Fixes #6168
Summary
This PR adds support for specifying custom instruction file paths outside the default
.roocode/folder, as requested in issue #6168. The implementation supports not only single file paths but also directories and glob patterns, as suggested by @snoyiatk.Features
.github/copilot-instructions.md).mdand.txtfiles from a directorydocs/**/*.mdfor flexible file selectionImplementation Details
customInstructionPathsfield to the global settings schemaloadCustomInstructionFilesfunction incustom-instructions.tsTesting
Usage Example
Users can now configure custom instruction paths in their settings:
{ "customInstructionPaths": [ ".github/copilot-instructions.md", "docs/guidelines/", "**/*.instructions.md" ] }Notes
Important
Adds support for custom instruction file paths with security validation and integration into system prompts, including comprehensive testing.
global-settings.tswithcustomInstructionPathsfield.custom-instructions.ts.loadCustomInstructionFiles()incustom-instructions.tshandles loading and validation of custom paths.addCustomInstructions()incustom-instructions.tsandgeneratePrompt()insystem.ts.custom-instructions.test.tsfor various scenarios including security and error handling.Task.tsandClineProvider.tsupdated to passcustomInstructionPathsto relevant functions.This description was created by
for 04b7823. You can customize this summary. It will automatically update as commits are pushed.