mirror of
https://github.com/RooVetGit/Roo-Code.git
synced 2026-08-28 05:27:24 +00:00
feat: add Orchestrator prompt guidance for permissions and UI visibility
- Enhance Orchestrator customInstructions with guidance on using the permissions parameter (filePatterns, commandPatterns, allowedTools, deniedTools) including example use cases and most-restrictive-wins semantics explanation - Add permission boundaries display in the ChatRow newTask approval message so users can see what restrictions are being set before approving subtask creation - Add i18n translation keys for permission display - Add 8 new tests across packages/types and webview-ui
This commit is contained in:
parent
caf552194f
commit
2bc25eb484
6 changed files with 261 additions and 2 deletions
|
|
@ -0,0 +1,32 @@
|
|||
import { describe, it, expect } from "vitest"
|
||||
import { DEFAULT_MODES } from "../mode.js"
|
||||
import type { ModeConfig } from "../mode.js"
|
||||
|
||||
describe("Orchestrator mode - permissions prompt guidance", () => {
|
||||
const orchestratorMode = DEFAULT_MODES.find((m: ModeConfig) => m.slug === "orchestrator")
|
||||
|
||||
it("should have the orchestrator mode defined", () => {
|
||||
expect(orchestratorMode).toBeDefined()
|
||||
})
|
||||
|
||||
it("should include permissions guidance in customInstructions", () => {
|
||||
expect(orchestratorMode!.customInstructions).toContain("permissions")
|
||||
expect(orchestratorMode!.customInstructions).toContain("filePatterns")
|
||||
expect(orchestratorMode!.customInstructions).toContain("commandPatterns")
|
||||
expect(orchestratorMode!.customInstructions).toContain("allowedTools")
|
||||
expect(orchestratorMode!.customInstructions).toContain("deniedTools")
|
||||
})
|
||||
|
||||
it("should mention most-restrictive-wins semantics", () => {
|
||||
expect(orchestratorMode!.customInstructions).toContain("most-restrictive-wins")
|
||||
})
|
||||
|
||||
it("should provide example use cases for permissions", () => {
|
||||
// Guidance about restricting file access
|
||||
expect(orchestratorMode!.customInstructions).toContain("specific directory")
|
||||
// Guidance about read-only research tasks
|
||||
expect(orchestratorMode!.customInstructions).toContain("read-only research")
|
||||
// Guidance about blocking shell access
|
||||
expect(orchestratorMode!.customInstructions).toContain("shell access")
|
||||
})
|
||||
})
|
||||
|
|
@ -222,6 +222,6 @@ export const DEFAULT_MODES: readonly ModeConfig[] = [
|
|||
description: "Coordinate tasks across multiple modes",
|
||||
groups: [],
|
||||
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.",
|
||||
'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\n8. When delegating subtasks, consider using the optional `permissions` parameter on `new_task` to restrict what the subtask can do. This is especially useful when:\n * The subtask should only modify files in a specific directory (use `filePatterns`, e.g. `["src/components/.*"]`).\n * The subtask should only run certain commands (use `commandPatterns`, e.g. `["npm test.*", "npm run lint"]`).\n * The subtask should be limited to specific tools (use `allowedTools`, e.g. `["read_file", "search_files"]` for read-only research tasks).\n * Certain tools should be explicitly blocked (use `deniedTools`, e.g. `["execute_command"]` to prevent shell access).\n Permissions are enforced at runtime and follow most-restrictive-wins semantics when subtasks are nested. Use them to keep subtasks focused and safe.\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.',
|
||||
},
|
||||
] as const
|
||||
|
|
|
|||
|
|
@ -795,6 +795,13 @@ export interface ClineSayTool {
|
|||
description?: string
|
||||
// Properties for skill tool
|
||||
skill?: string
|
||||
// Properties for newTask tool - permission boundaries set by parent
|
||||
permissions?: {
|
||||
filePatterns?: string[]
|
||||
commandPatterns?: string[]
|
||||
allowedTools?: string[]
|
||||
deniedTools?: string[]
|
||||
}
|
||||
}
|
||||
|
||||
export interface ClineAskUseMcpServer {
|
||||
|
|
|
|||
|
|
@ -869,6 +869,39 @@ export const ChatRowContent = ({
|
|||
</div>
|
||||
<div className="border-l border-muted-foreground/80 ml-2 pl-4 pb-1">
|
||||
<MarkdownBlock markdown={tool.content} />
|
||||
{tool.permissions && (
|
||||
<div className="mt-2 p-2 rounded text-xs text-vscode-descriptionForeground bg-vscode-editor-background border border-vscode-editorGroup-border">
|
||||
<div className="font-semibold mb-1">{t("chat:subtasks.permissionBoundaries")}</div>
|
||||
{tool.permissions.filePatterns && (
|
||||
<div>
|
||||
{t("chat:subtasks.permissionFilePatterns", {
|
||||
patterns: tool.permissions.filePatterns.join(", "),
|
||||
})}
|
||||
</div>
|
||||
)}
|
||||
{tool.permissions.commandPatterns && (
|
||||
<div>
|
||||
{t("chat:subtasks.permissionCommandPatterns", {
|
||||
patterns: tool.permissions.commandPatterns.join(", "),
|
||||
})}
|
||||
</div>
|
||||
)}
|
||||
{tool.permissions.allowedTools && (
|
||||
<div>
|
||||
{t("chat:subtasks.permissionAllowedTools", {
|
||||
tools: tool.permissions.allowedTools.join(", "),
|
||||
})}
|
||||
</div>
|
||||
)}
|
||||
{tool.permissions.deniedTools && (
|
||||
<div>
|
||||
{t("chat:subtasks.permissionDeniedTools", {
|
||||
tools: tool.permissions.deniedTools.join(", "),
|
||||
})}
|
||||
</div>
|
||||
)}
|
||||
</div>
|
||||
)}
|
||||
<div>
|
||||
{childTaskId && !isFollowedBySubtaskResult && (
|
||||
<button
|
||||
|
|
|
|||
|
|
@ -0,0 +1,182 @@
|
|||
import React from "react"
|
||||
import { render, screen } from "@/utils/test-utils"
|
||||
import { QueryClient, QueryClientProvider } from "@tanstack/react-query"
|
||||
import { ChatRowContent } from "../ChatRow"
|
||||
import type { HistoryItem, ClineMessage } from "@roo-code/types"
|
||||
|
||||
// Mock vscode API
|
||||
const mockPostMessage = vi.fn()
|
||||
vi.mock("@src/utils/vscode", () => ({
|
||||
vscode: {
|
||||
postMessage: (msg: unknown) => mockPostMessage(msg),
|
||||
},
|
||||
}))
|
||||
|
||||
// Mock i18n - return key-based strings so we can assert on the right keys
|
||||
vi.mock("react-i18next", () => ({
|
||||
useTranslation: () => ({
|
||||
t: (key: string, params?: Record<string, string>) => {
|
||||
const map: Record<string, string> = {
|
||||
"chat:subtasks.wantsToCreate": "Roo wants to create a new subtask",
|
||||
"chat:subtasks.permissionBoundaries": "Permission Boundaries",
|
||||
"chat:subtasks.goToSubtask": "Go to subtask",
|
||||
}
|
||||
if (key === "chat:subtasks.permissionFilePatterns" && params?.patterns) {
|
||||
return `Allowed files: ${params.patterns}`
|
||||
}
|
||||
if (key === "chat:subtasks.permissionCommandPatterns" && params?.patterns) {
|
||||
return `Allowed commands: ${params.patterns}`
|
||||
}
|
||||
if (key === "chat:subtasks.permissionAllowedTools" && params?.tools) {
|
||||
return `Allowed tools: ${params.tools}`
|
||||
}
|
||||
if (key === "chat:subtasks.permissionDeniedTools" && params?.tools) {
|
||||
return `Denied tools: ${params.tools}`
|
||||
}
|
||||
return map[key] ?? key
|
||||
},
|
||||
i18n: { exists: () => true },
|
||||
}),
|
||||
Trans: ({ children }: { children?: React.ReactNode }) => <>{children}</>,
|
||||
initReactI18next: { type: "3rdParty", init: () => {} },
|
||||
}))
|
||||
|
||||
// Mock extension state context
|
||||
let mockCurrentTaskItem: Partial<HistoryItem> | undefined = undefined
|
||||
let mockClineMessages: ClineMessage[] = []
|
||||
|
||||
vi.mock("@src/context/ExtensionStateContext", () => ({
|
||||
useExtensionState: () => ({
|
||||
mcpServers: [],
|
||||
alwaysAllowMcp: false,
|
||||
currentCheckpoint: null,
|
||||
mode: "code",
|
||||
apiConfiguration: {},
|
||||
clineMessages: mockClineMessages,
|
||||
currentTaskItem: mockCurrentTaskItem,
|
||||
}),
|
||||
}))
|
||||
|
||||
// Mock useSelectedModel hook
|
||||
vi.mock("@src/components/ui/hooks/useSelectedModel", () => ({
|
||||
useSelectedModel: () => ({ info: { supportsImages: true } }),
|
||||
}))
|
||||
|
||||
const queryClient = new QueryClient()
|
||||
|
||||
function renderChatRow(message: any, currentTaskItem?: Partial<HistoryItem>, clineMessages?: ClineMessage[]) {
|
||||
mockCurrentTaskItem = currentTaskItem
|
||||
mockClineMessages = clineMessages || [message]
|
||||
|
||||
return render(
|
||||
<QueryClientProvider client={queryClient}>
|
||||
<ChatRowContent
|
||||
message={message}
|
||||
isExpanded={false}
|
||||
isLast={false}
|
||||
isStreaming={false}
|
||||
onToggleExpand={() => {}}
|
||||
onSuggestionClick={() => {}}
|
||||
onBatchFileResponse={() => {}}
|
||||
onFollowUpUnmount={() => {}}
|
||||
isFollowUpAnswered={false}
|
||||
/>
|
||||
</QueryClientProvider>,
|
||||
)
|
||||
}
|
||||
|
||||
describe("ChatRow - permission boundaries display", () => {
|
||||
beforeEach(() => {
|
||||
mockPostMessage.mockClear()
|
||||
})
|
||||
|
||||
it("should display permission boundaries when permissions are set on a newTask", () => {
|
||||
const message = {
|
||||
ts: Date.now(),
|
||||
type: "ask" as const,
|
||||
ask: "tool" as const,
|
||||
text: JSON.stringify({
|
||||
tool: "newTask",
|
||||
mode: "code",
|
||||
content: "Edit the Button component",
|
||||
permissions: {
|
||||
filePatterns: ["src/components/.*"],
|
||||
commandPatterns: ["npm test.*"],
|
||||
deniedTools: ["execute_command"],
|
||||
},
|
||||
}),
|
||||
}
|
||||
|
||||
renderChatRow(message)
|
||||
|
||||
expect(screen.getByText("Permission Boundaries")).toBeInTheDocument()
|
||||
expect(screen.getByText("Allowed files: src/components/.*")).toBeInTheDocument()
|
||||
expect(screen.getByText("Allowed commands: npm test.*")).toBeInTheDocument()
|
||||
expect(screen.getByText("Denied tools: execute_command")).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it("should display allowedTools when set", () => {
|
||||
const message = {
|
||||
ts: Date.now(),
|
||||
type: "ask" as const,
|
||||
ask: "tool" as const,
|
||||
text: JSON.stringify({
|
||||
tool: "newTask",
|
||||
mode: "ask",
|
||||
content: "Research the API",
|
||||
permissions: {
|
||||
allowedTools: ["read_file", "search_files", "codebase_search"],
|
||||
},
|
||||
}),
|
||||
}
|
||||
|
||||
renderChatRow(message)
|
||||
|
||||
expect(screen.getByText("Permission Boundaries")).toBeInTheDocument()
|
||||
expect(screen.getByText("Allowed tools: read_file, search_files, codebase_search")).toBeInTheDocument()
|
||||
})
|
||||
|
||||
it("should not display permission boundaries when permissions are not set", () => {
|
||||
const message = {
|
||||
ts: Date.now(),
|
||||
type: "ask" as const,
|
||||
ask: "tool" as const,
|
||||
text: JSON.stringify({
|
||||
tool: "newTask",
|
||||
mode: "code",
|
||||
content: "Implement feature X",
|
||||
}),
|
||||
}
|
||||
|
||||
renderChatRow(message)
|
||||
|
||||
expect(screen.queryByText("Permission Boundaries")).not.toBeInTheDocument()
|
||||
})
|
||||
|
||||
it("should display multiple permission types together", () => {
|
||||
const message = {
|
||||
ts: Date.now(),
|
||||
type: "ask" as const,
|
||||
ask: "tool" as const,
|
||||
text: JSON.stringify({
|
||||
tool: "newTask",
|
||||
mode: "code",
|
||||
content: "Edit and test components",
|
||||
permissions: {
|
||||
filePatterns: ["src/components/.*", "src/utils/.*"],
|
||||
commandPatterns: ["npm test.*", "npm run lint"],
|
||||
allowedTools: ["read_file", "write_to_file", "execute_command"],
|
||||
deniedTools: ["apply_patch"],
|
||||
},
|
||||
}),
|
||||
}
|
||||
|
||||
renderChatRow(message)
|
||||
|
||||
expect(screen.getByText("Permission Boundaries")).toBeInTheDocument()
|
||||
expect(screen.getByText("Allowed files: src/components/.*, src/utils/.*")).toBeInTheDocument()
|
||||
expect(screen.getByText("Allowed commands: npm test.*, npm run lint")).toBeInTheDocument()
|
||||
expect(screen.getByText("Allowed tools: read_file, write_to_file, execute_command")).toBeInTheDocument()
|
||||
expect(screen.getByText("Denied tools: apply_patch")).toBeInTheDocument()
|
||||
})
|
||||
})
|
||||
|
|
@ -313,7 +313,12 @@
|
|||
"filesModified": "Files Modified",
|
||||
"filesRead": "Files Read",
|
||||
"commandsExecuted": "Commands Executed",
|
||||
"todoStats": "Todos: {{completed}}/{{total}} completed"
|
||||
"todoStats": "Todos: {{completed}}/{{total}} completed",
|
||||
"permissionBoundaries": "Permission Boundaries",
|
||||
"permissionFilePatterns": "Allowed files: {{patterns}}",
|
||||
"permissionCommandPatterns": "Allowed commands: {{patterns}}",
|
||||
"permissionAllowedTools": "Allowed tools: {{tools}}",
|
||||
"permissionDeniedTools": "Denied tools: {{tools}}"
|
||||
},
|
||||
"questions": {
|
||||
"hasQuestion": "Roo has a question"
|
||||
|
|
|
|||
Loading…
Add table
Reference in a new issue