Skip to content

Commit 5f8a888

Browse files
mrubensmrubens
committed
Validation fixes
1 parent 569e104 commit 5f8a888

File tree

6 files changed

+109
-99
lines changed

6 files changed

+109
-99
lines changed

‎src/core/Cline.ts‎

Lines changed: 19 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -1142,17 +1142,25 @@ export class Cline {
11421142
await this.browserSession.closeBrowser()
11431143
}
11441144

1145-
// Validate tool use before execution
1146-
const { mode } = (await this.providerRef.deref()?.getState()) ?? {}
1147-
const { customModes } = (await this.providerRef.deref()?.getState()) ?? {}
1148-
try {
1149-
validateToolUse(block.name as ToolName, mode ?? defaultModeSlug, customModes ?? [], {
1150-
apply_diff: this.diffEnabled,
1151-
})
1152-
} catch (error) {
1153-
this.consecutiveMistakeCount++
1154-
pushToolResult(formatResponse.toolError(error.message))
1155-
break
1145+
// Only validate complete tool uses
1146+
if (!block.partial) {
1147+
const { mode } = (await this.providerRef.deref()?.getState()) ?? {}
1148+
const { customModes } = (await this.providerRef.deref()?.getState()) ?? {}
1149+
try {
1150+
validateToolUse(
1151+
block.name as ToolName,
1152+
mode ?? defaultModeSlug,
1153+
customModes ?? [],
1154+
{
1155+
apply_diff: this.diffEnabled,
1156+
},
1157+
block.params,
1158+
)
1159+
} catch (error) {
1160+
this.consecutiveMistakeCount++
1161+
pushToolResult(formatResponse.toolError(error.message))
1162+
break
1163+
}
11561164
}
11571165

11581166
switch (block.name) {

‎src/core/config/CustomModesSchema.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@ const GroupOptionsSchema = z.object({
2222
},
2323
{ message: "Invalid regular expression pattern" },
2424
),
25-
fileRegexDescription: z.string().optional(),
25+
description: z.string().optional(),
2626
})
2727

2828
// Schema for a group entry - either a tool group string or a tuple of [group, options]

‎src/core/config/__tests__/CustomModesSchema.test.ts‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,7 @@ describe("CustomModeSchema", () => {
135135
roleDefinition: "Documentation editing mode",
136136
groups: [
137137
"read",
138-
["edit", { fileRegex: "\\.(md|txt)$", fileRegexDescription: "Documentation files only" }],
138+
["edit", { fileRegex: "\\.(md|txt)$", description: "Documentation files only" }],
139139
"browser",
140140
],
141141
}

‎src/core/mode-validator.ts‎

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,4 +1,4 @@
1-
import { Mode, isToolAllowedForMode, getModeConfig, ModeConfig } from "../shared/modes"
1+
import { Mode, isToolAllowedForMode, getModeConfig, ModeConfig, FileRestrictionError } from "../shared/modes"
22
import { ToolName } from "../shared/tool-groups"
33

44
export { isToolAllowedForMode }
@@ -9,8 +9,9 @@ export function validateToolUse(
99
mode: Mode,
1010
customModes?: ModeConfig[],
1111
toolRequirements?: Record<string, boolean>,
12+
toolParams?: Record<string, unknown>,
1213
): void {
13-
if (!isToolAllowedForMode(toolName, mode, customModes ?? [], toolRequirements)) {
14+
if (!isToolAllowedForMode(toolName, mode, customModes ?? [], toolRequirements, toolParams)) {
1415
throw new Error(`Tool "${toolName}" is not allowed in ${mode} mode.`)
1516
}
1617
}

‎src/shared/__tests__/modes.test.ts‎

Lines changed: 77 additions & 78 deletions
Original file line numberDiff line numberDiff line change
@@ -29,59 +29,57 @@ describe("isToolAllowedForMode", () => {
2929
describe("file restrictions", () => {
3030
it("allows editing matching files", () => {
3131
// Test markdown editor mode
32-
const mdResult = isToolAllowedForMode("write_to_file", "markdown-editor", customModes, undefined, "test.md")
32+
const mdResult = isToolAllowedForMode("write_to_file", "markdown-editor", customModes, undefined, {
33+
path: "test.md",
34+
})
3335
expect(mdResult).toBe(true)
3436

3537
// Test CSS editor mode
36-
const cssResult = isToolAllowedForMode("write_to_file", "css-editor", customModes, undefined, "styles.css")
38+
const cssResult = isToolAllowedForMode("write_to_file", "css-editor", customModes, undefined, {
39+
path: "styles.css",
40+
})
3741
expect(cssResult).toBe(true)
3842
})
3943

4044
it("rejects editing non-matching files", () => {
4145
// Test markdown editor mode with non-markdown file
42-
const mdError = isToolAllowedForMode("write_to_file", "markdown-editor", customModes, undefined, "test.js")
43-
expect(mdError).toBeInstanceOf(FileRestrictionError)
44-
expect((mdError as FileRestrictionError).message).toContain("\\.md$")
46+
expect(() =>
47+
isToolAllowedForMode("write_to_file", "markdown-editor", customModes, undefined, { path: "test.js" }),
48+
).toThrow(FileRestrictionError)
49+
expect(() =>
50+
isToolAllowedForMode("write_to_file", "markdown-editor", customModes, undefined, { path: "test.js" }),
51+
).toThrow(/\\.md\$/)
4552

4653
// Test CSS editor mode with non-CSS file
47-
const cssError = isToolAllowedForMode("write_to_file", "css-editor", customModes, undefined, "test.js")
48-
expect(cssError).toBeInstanceOf(FileRestrictionError)
49-
expect((cssError as FileRestrictionError).message).toContain("\\.css$")
50-
})
51-
52-
it("requires file path for restricted edit operations", () => {
53-
const result = isToolAllowedForMode("write_to_file", "markdown-editor", customModes)
54-
expect(result).toBeInstanceOf(FileRestrictionError)
55-
expect((result as FileRestrictionError).message).toContain("\\.md$")
54+
expect(() =>
55+
isToolAllowedForMode("write_to_file", "css-editor", customModes, undefined, { path: "test.js" }),
56+
).toThrow(FileRestrictionError)
57+
expect(() =>
58+
isToolAllowedForMode("write_to_file", "css-editor", customModes, undefined, { path: "test.js" }),
59+
).toThrow(/\\.css\$/)
5660
})
5761

5862
it("applies restrictions to both write_to_file and apply_diff", () => {
5963
// Test write_to_file
60-
const writeResult = isToolAllowedForMode(
61-
"write_to_file",
62-
"markdown-editor",
63-
customModes,
64-
undefined,
65-
"test.md",
66-
)
64+
const writeResult = isToolAllowedForMode("write_to_file", "markdown-editor", customModes, undefined, {
65+
path: "test.md",
66+
})
6767
expect(writeResult).toBe(true)
6868

6969
// Test apply_diff
70-
const diffResult = isToolAllowedForMode("apply_diff", "markdown-editor", customModes, undefined, "test.md")
70+
const diffResult = isToolAllowedForMode("apply_diff", "markdown-editor", customModes, undefined, {
71+
path: "test.md",
72+
})
7173
expect(diffResult).toBe(true)
7274

7375
// Test both with non-matching file
74-
const writeError = isToolAllowedForMode(
75-
"write_to_file",
76-
"markdown-editor",
77-
customModes,
78-
undefined,
79-
"test.js",
80-
)
81-
expect(writeError).toBeInstanceOf(FileRestrictionError)
76+
expect(() =>
77+
isToolAllowedForMode("write_to_file", "markdown-editor", customModes, undefined, { path: "test.js" }),
78+
).toThrow(FileRestrictionError)
8279

83-
const diffError = isToolAllowedForMode("apply_diff", "markdown-editor", customModes, undefined, "test.js")
84-
expect(diffError).toBeInstanceOf(FileRestrictionError)
80+
expect(() =>
81+
isToolAllowedForMode("apply_diff", "markdown-editor", customModes, undefined, { path: "test.js" }),
82+
).toThrow(FileRestrictionError)
8583
})
8684

8785
it("uses description in file restriction error for custom modes", () => {
@@ -99,60 +97,57 @@ describe("isToolAllowedForMode", () => {
9997
]
10098

10199
// Test write_to_file with non-matching file
102-
const writeError = isToolAllowedForMode(
103-
"write_to_file",
104-
"docs-editor",
105-
customModesWithDescription,
106-
undefined,
107-
"test.js",
108-
)
109-
expect(writeError).toBeInstanceOf(FileRestrictionError)
110-
expect((writeError as FileRestrictionError).message).toContain("Documentation files only")
100+
expect(() =>
101+
isToolAllowedForMode("write_to_file", "docs-editor", customModesWithDescription, undefined, {
102+
path: "test.js",
103+
}),
104+
).toThrow(FileRestrictionError)
105+
expect(() =>
106+
isToolAllowedForMode("write_to_file", "docs-editor", customModesWithDescription, undefined, {
107+
path: "test.js",
108+
}),
109+
).toThrow(/Documentation files only/)
111110

112111
// Test apply_diff with non-matching file
113-
const diffError = isToolAllowedForMode(
114-
"apply_diff",
115-
"docs-editor",
116-
customModesWithDescription,
117-
undefined,
118-
"test.js",
119-
)
120-
expect(diffError).toBeInstanceOf(FileRestrictionError)
121-
expect((diffError as FileRestrictionError).message).toContain("Documentation files only")
112+
expect(() =>
113+
isToolAllowedForMode("apply_diff", "docs-editor", customModesWithDescription, undefined, {
114+
path: "test.js",
115+
}),
116+
).toThrow(FileRestrictionError)
117+
expect(() =>
118+
isToolAllowedForMode("apply_diff", "docs-editor", customModesWithDescription, undefined, {
119+
path: "test.js",
120+
}),
121+
).toThrow(/Documentation files only/)
122122

123123
// Test that matching files are allowed
124-
const mdResult = isToolAllowedForMode(
125-
"write_to_file",
126-
"docs-editor",
127-
customModesWithDescription,
128-
undefined,
129-
"test.md",
130-
)
131-
expect(mdResult).toBe(true)
132-
133-
const txtResult = isToolAllowedForMode(
134-
"write_to_file",
135-
"docs-editor",
136-
customModesWithDescription,
137-
undefined,
138-
"test.txt",
139-
)
140-
expect(txtResult).toBe(true)
124+
expect(
125+
isToolAllowedForMode("write_to_file", "docs-editor", customModesWithDescription, undefined, {
126+
path: "test.md",
127+
}),
128+
).toBe(true)
129+
130+
expect(
131+
isToolAllowedForMode("write_to_file", "docs-editor", customModesWithDescription, undefined, {
132+
path: "test.txt",
133+
}),
134+
).toBe(true)
141135
})
142136

143137
it("allows ask mode to edit markdown files only", () => {
144138
// Should allow editing markdown files
145-
const mdResult = isToolAllowedForMode("write_to_file", "ask", [], undefined, "test.md")
146-
expect(mdResult).toBe(true)
139+
expect(isToolAllowedForMode("write_to_file", "ask", [], undefined, { path: "test.md" })).toBe(true)
147140

148141
// Should allow applying diffs to markdown files
149-
const diffResult = isToolAllowedForMode("apply_diff", "ask", [], undefined, "readme.md")
150-
expect(diffResult).toBe(true)
142+
expect(isToolAllowedForMode("apply_diff", "ask", [], undefined, { path: "readme.md" })).toBe(true)
151143

152144
// Should reject non-markdown files
153-
const jsResult = isToolAllowedForMode("write_to_file", "ask", [], undefined, "test.js")
154-
expect(jsResult).toBeInstanceOf(FileRestrictionError)
155-
expect((jsResult as FileRestrictionError).message).toContain("Markdown files only")
145+
expect(() => isToolAllowedForMode("write_to_file", "ask", [], undefined, { path: "test.js" })).toThrow(
146+
FileRestrictionError,
147+
)
148+
expect(() => isToolAllowedForMode("write_to_file", "ask", [], undefined, { path: "test.js" })).toThrow(
149+
/Markdown files only/,
150+
)
156151

157152
// Should maintain read capabilities
158153
expect(isToolAllowedForMode("read_file", "ask", [])).toBe(true)
@@ -176,14 +171,18 @@ describe("isToolAllowedForMode", () => {
176171

177172
describe("FileRestrictionError", () => {
178173
it("formats error message with pattern when no description provided", () => {
179-
const error = new FileRestrictionError("Markdown Editor", "\\.md$")
180-
expect(error.message).toBe("This mode (Markdown Editor) can only edit files matching the pattern: \\.md$")
174+
const error = new FileRestrictionError("Markdown Editor", "\\.md$", undefined, "test.js")
175+
expect(error.message).toBe(
176+
"This mode (Markdown Editor) can only edit files matching pattern: \\.md$. Got: test.js",
177+
)
181178
expect(error.name).toBe("FileRestrictionError")
182179
})
183180

184181
it("formats error message with description when provided", () => {
185-
const error = new FileRestrictionError("Markdown Editor", "\\.md$", "Markdown files only")
186-
expect(error.message).toBe("This mode (Markdown Editor) can only edit files matching Markdown files only")
182+
const error = new FileRestrictionError("Markdown Editor", "\\.md$", "Markdown files only", "test.js")
183+
expect(error.message).toBe(
184+
"This mode (Markdown Editor) can only edit files matching pattern: \\.md$ (Markdown files only). Got: test.js",
185+
)
187186
expect(error.name).toBe("FileRestrictionError")
188187
})
189188
})

‎src/shared/modes.ts‎

Lines changed: 8 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -146,9 +146,9 @@ export function isCustomMode(slug: string, customModes?: ModeConfig[]): boolean
146146

147147
// Custom error class for file restrictions
148148
export class FileRestrictionError extends Error {
149-
constructor(mode: string, pattern: string, description?: string) {
149+
constructor(mode: string, pattern: string, description: string | undefined, filePath: string) {
150150
super(
151-
`This mode (${mode}) can only edit files matching ${description ? description : `the pattern: ${pattern}`}`,
151+
`This mode (${mode}) can only edit files matching pattern: ${pattern}${description ? ` (${description})` : ""}. Got: ${filePath}`,
152152
)
153153
this.name = "FileRestrictionError"
154154
}
@@ -159,8 +159,8 @@ export function isToolAllowedForMode(
159159
modeSlug: string,
160160
customModes: ModeConfig[],
161161
toolRequirements?: Record<string, boolean>,
162-
filePath?: string, // Optional file path for checking regex patterns
163-
): boolean | FileRestrictionError {
162+
toolParams?: Record<string, any>, // All tool parameters
163+
): boolean {
164164
// Always allow these tools
165165
if (ALWAYS_AVAILABLE_TOOLS.includes(tool as any)) {
166166
return true
@@ -195,8 +195,10 @@ export function isToolAllowedForMode(
195195

196196
// For the edit group, check file regex if specified
197197
if (groupName === "edit" && options.fileRegex) {
198-
if (!filePath || !doesFileMatchRegex(filePath, options.fileRegex)) {
199-
return new FileRestrictionError(mode.name, options.fileRegex, options.description)
198+
const filePath = toolParams?.path
199+
// Only validate regex if a path is provided
200+
if (filePath && !doesFileMatchRegex(filePath, options.fileRegex)) {
201+
throw new FileRestrictionError(mode.name, options.fileRegex, options.description, filePath)
200202
}
201203
return true
202204
}

0 commit comments

Comments
 (0)