feat(modes): support fileRegex for read_file and allow orchestrator skill reads

This commit is contained in:
Hannes Rudolph 2025-12-30 15:06:31 -07:00
parent c37aa02b21
commit ceb026efd9
6 changed files with 184 additions and 4 deletions

View file

@ -188,7 +188,15 @@ export const DEFAULT_MODES: readonly ModeConfig[] = [
whenToUse:
"Use this mode for complex, multi-step projects that require coordination across different specialties. Ideal when you need to break down large tasks into subtasks, manage workflows, or coordinate work that spans multiple domains or expertise areas.",
description: "Coordinate tasks across multiple modes",
groups: [],
groups: [
[
"read",
{
fileRegex: "^\\.roo\\/skills(-[a-zA-Z0-9-]+)?\\/[^\\/]+\\/SKILL\\.md$",
description: "Skill definition files only",
},
],
],
customInstructions:
"Your role is to coordinate complex workflows by delegating tasks to specialized modes. As an orchestrator, you should:\n\n1. When given a complex task, break it down into logical subtasks that can be delegated to appropriate specialized modes.\n\n2. For each subtask, use the `new_task` tool to delegate. Choose the most appropriate mode for the subtask's specific goal and provide comprehensive instructions in the `message` parameter. These instructions must include:\n * All necessary context from the parent task or previous subtasks required to complete the work.\n * A clearly defined scope, specifying exactly what the subtask should accomplish.\n * An explicit statement that the subtask should *only* perform the work outlined in these instructions and not deviate.\n * An instruction for the subtask to signal completion by using the `attempt_completion` tool, providing a concise yet thorough summary of the outcome in the `result` parameter, keeping in mind that this summary will be the source of truth used to keep track of what was completed on this project.\n * A statement that these specific instructions supersede any conflicting general instructions the subtask's mode might have.\n\n3. Track and manage the progress of all subtasks. When a subtask is completed, analyze its results and determine the next steps.\n\n4. Help the user understand how the different subtasks fit together in the overall workflow. Provide clear reasoning about why you're delegating specific tasks to specific modes.\n\n5. When all subtasks are completed, synthesize the results and provide a comprehensive overview of what was accomplished.\n\n6. Ask clarifying questions when necessary to better understand how to break down complex tasks effectively.\n\n7. Suggest improvements to the workflow based on the results of completed subtasks.\n\nUse subtasks to maintain clarity. If a request significantly shifts focus or requires a different expertise (mode), consider creating a subtask rather than overloading the current one.",
},

View file

@ -714,13 +714,19 @@ export async function presentAssistantMessage(cline: Task) {
const { resolveToolAlias } = await import("../prompts/tools/filter-tools-for-mode")
const includedTools = rawIncludedTools?.map((tool) => resolveToolAlias(tool))
// Prefer nativeArgs for validation when available (e.g., read_file uses nativeArgs.files).
// nativeArgs should win on key collisions.
const toolParamsForValidation = block.nativeArgs
? { ...block.params, ...block.nativeArgs }
: block.params
try {
validateToolUse(
block.name as ToolName,
mode ?? defaultModeSlug,
customModes ?? [],
{ apply_diff: cline.diffEnabled },
block.params,
toolParamsForValidation,
stateExperiments,
includedTools,
)

View file

@ -185,6 +185,27 @@ describe("mode-validator", () => {
expect(() => validateToolUse("read_file", "architect", [])).not.toThrow()
})
it("enforces read fileRegex restrictions for read_file", () => {
const customModes: ModeConfig[] = [
{
slug: "md-reader",
name: "Markdown Reader",
roleDefinition: "Read markdown only",
groups: [["read", { fileRegex: "\\.md$" }]] as const,
},
]
expect(() =>
validateToolUse("read_file", "md-reader", customModes, undefined, {
files: [{ path: "src/index.ts" }],
}),
).toThrow(/can only read files matching pattern/)
expect(() =>
validateToolUse("read_file", "md-reader", customModes, undefined, { files: [{ path: "README.md" }] }),
).not.toThrow()
})
it("throws error when tool requirement is not met", () => {
const requirements = { apply_diff: false }
expect(() => validateToolUse("apply_diff", codeMode, [], requirements)).toThrow(

View file

@ -78,6 +78,52 @@ function doesFileMatchRegex(filePath: string, pattern: string): boolean {
}
}
function extractReadFilePaths(toolParams: Record<string, unknown> | undefined): string[] {
if (!toolParams) {
return []
}
const paths: string[] = []
// Native protocol read_file: { files: [{ path: string }] }
const files = (toolParams as { files?: unknown }).files
if (Array.isArray(files)) {
for (const entry of files) {
if (typeof entry === "object" && entry !== null) {
const p = (entry as { path?: unknown }).path
if (typeof p === "string" && p.trim().length > 0) {
paths.push(p.trim())
}
}
}
}
// Legacy single-path read_file: { path: string }
const legacyPath = (toolParams as { path?: unknown }).path
if (typeof legacyPath === "string" && legacyPath.trim().length > 0) {
paths.push(legacyPath.trim())
}
// Legacy XML args read_file: { args: "<args><file><path>...</path>...</file></args>" }
const args = (toolParams as { args?: unknown }).args
if (typeof args === "string") {
const filePathMatches = args.match(/<path>([^<]+)<\/path>/g)
if (filePathMatches) {
for (const match of filePathMatches) {
const pathMatch = match.match(/<path>([^<]+)<\/path>/)
if (pathMatch && pathMatch[1]) {
const extractedPath = pathMatch[1].trim()
if (extractedPath && !extractedPath.includes("<") && !extractedPath.includes(">")) {
paths.push(extractedPath)
}
}
}
}
}
return Array.from(new Set(paths))
}
export function isToolAllowedForMode(
tool: string,
modeSlug: string,
@ -201,6 +247,16 @@ export function isToolAllowedForMode(
}
}
// For the read group, optionally restrict read_file paths if specified
if (groupName === "read" && options.fileRegex && tool === "read_file") {
const readPaths = extractReadFilePaths(toolParams)
for (const p of readPaths) {
if (!doesFileMatchRegex(p, options.fileRegex)) {
throw new FileRestrictionError(mode.name, options.fileRegex, options.description, p, tool, "read")
}
}
}
return true
}

View file

@ -45,6 +45,52 @@ describe("isToolAllowedForMode", () => {
expect(isToolAllowedForMode("browser_action", "markdown-editor", customModes)).toBe(true)
})
describe("read file restrictions", () => {
it("allows reading matching files", () => {
const customModesWithReadRestriction: ModeConfig[] = [
{
slug: "md-reader",
name: "Markdown Reader",
roleDefinition: "You can only read markdown",
groups: ["browser", ["read", { fileRegex: "\\.md$" }]],
},
]
expect(
isToolAllowedForMode("read_file", "md-reader", customModesWithReadRestriction, undefined, {
files: [{ path: "README.md" }],
}),
).toBe(true)
})
it("rejects reading non-matching files", () => {
const customModesWithReadRestriction: ModeConfig[] = [
{
slug: "md-reader",
name: "Markdown Reader",
roleDefinition: "You can only read markdown",
groups: [["read", { fileRegex: "\\.md$", description: "Markdown files only" }]],
},
]
expect(() =>
isToolAllowedForMode("read_file", "md-reader", customModesWithReadRestriction, undefined, {
files: [{ path: "src/index.ts" }],
}),
).toThrow(FileRestrictionError)
expect(() =>
isToolAllowedForMode("read_file", "md-reader", customModesWithReadRestriction, undefined, {
files: [{ path: "src/index.ts" }],
}),
).toThrow(/can only read files matching pattern/)
expect(() =>
isToolAllowedForMode("read_file", "md-reader", customModesWithReadRestriction, undefined, {
files: [{ path: "src/index.ts" }],
}),
).toThrow(/Markdown files only/)
})
})
describe("file restrictions", () => {
it("allows editing matching files", () => {
// Test markdown editor mode
@ -395,6 +441,13 @@ describe("FileRestrictionError", () => {
expect(error.name).toBe("FileRestrictionError")
})
it("formats error message for read operations", () => {
const error = new FileRestrictionError("Markdown Reader", "\\.md$", undefined, "test.js", "read_file", "read")
expect(error.message).toBe(
"Tool 'read_file' in mode 'Markdown Reader' can only read files matching pattern: \\.md$. Got: test.js",
)
})
describe("debug mode", () => {
it("is configured correctly", () => {
const debugMode = modes.find((mode) => mode.slug === "debug")
@ -412,6 +465,34 @@ describe("FileRestrictionError", () => {
})
})
describe("orchestrator mode", () => {
it("is configured to only read skill definition files", () => {
const orchestratorMode = modes.find((mode) => mode.slug === "orchestrator")
expect(orchestratorMode).toBeDefined()
expect(orchestratorMode?.groups).toEqual([
[
"read",
{
fileRegex: "^\\.roo\\/skills(-[a-zA-Z0-9-]+)?\\/[^\\/]+\\/SKILL\\.md$",
description: "Skill definition files only",
},
],
])
expect(
isToolAllowedForMode("read_file", "orchestrator", [], undefined, {
files: [{ path: ".roo/skills/example-skill/SKILL.md" }],
}),
).toBe(true)
expect(() =>
isToolAllowedForMode("read_file", "orchestrator", [], undefined, {
files: [{ path: "src/index.ts" }],
}),
).toThrow(FileRestrictionError)
})
})
describe("getFullModeDetails", () => {
beforeEach(() => {
vi.clearAllMocks()

View file

@ -133,10 +133,18 @@ export function getModeSelection(mode: string, promptComponent?: PromptComponent
// Custom error class for file restrictions
export class FileRestrictionError extends Error {
constructor(mode: string, pattern: string, description: string | undefined, filePath: string, tool?: string) {
constructor(
mode: string,
pattern: string,
description: string | undefined,
filePath: string,
tool?: string,
operation: "read" | "edit" = "edit",
) {
const toolInfo = tool ? `Tool '${tool}' in mode '${mode}'` : `This mode (${mode})`
const verb = operation === "read" ? "read" : "edit"
super(
`${toolInfo} can only edit files matching pattern: ${pattern}${description ? ` (${description})` : ""}. Got: ${filePath}`,
`${toolInfo} can only ${verb} files matching pattern: ${pattern}${description ? ` (${description})` : ""}. Got: ${filePath}`,
)
this.name = "FileRestrictionError"
}