Skip to content

Commit 2333c3e

Browse files
Smartsheet-JB-BrownSmartsheet-JB-Brown
committed
Merge branch 'main' into jbbrown/bedrock_caching
* main: Remove the ask promise error (RooCodeInc#2123) Tweaks to support prompt line number format (RooCodeInc#2121) Fix switching profiles to ensure only the selected profile is switche… (RooCodeInc#2119) Fix Gemini command escaping (RooCodeInc#2120) Parse the retry out of the gemini 429 response (RooCodeInc#2118) Revert "Remove the ask promise error" (RooCodeInc#2117)
2 parents a85b697 + b788aa1 commit 2333c3e

File tree

21 files changed

+339
-62
lines changed

21 files changed

+339
-62
lines changed

‎src/core/Cline.ts‎

Lines changed: 24 additions & 21 deletions
Original file line numberDiff line numberDiff line change
@@ -332,17 +332,7 @@ export class Cline extends EventEmitter<ClineEvents> {
332332
}
333333

334334
private async addToClineMessages(message: ClineMessage) {
335-
// Find the correct position to insert the message based on timestamp
336-
const insertIndex = this.clineMessages.findIndex((existingMsg) => existingMsg.ts > message.ts)
337-
338-
if (insertIndex === -1) {
339-
// If no message with a later timestamp is found, append to the end
340-
this.clineMessages.push(message)
341-
} else {
342-
// Insert the message at the correct position to maintain chronological order
343-
this.clineMessages.splice(insertIndex, 0, message)
344-
}
345-
335+
this.clineMessages.push(message)
346336
await this.providerRef.deref()?.postStateToWebview()
347337
this.emit("message", { action: "created", message })
348338
await this.saveClineMessages()
@@ -519,8 +509,6 @@ export class Cline extends EventEmitter<ClineEvents> {
519509
throw new Error(`[Cline#say] task ${this.taskId}.${this.instanceId} aborted`)
520510
}
521511

522-
const sayTs = (checkpoint?.startTime as number) ?? Date.now()
523-
524512
if (partial !== undefined) {
525513
const lastMessage = this.clineMessages.at(-1)
526514
const isUpdatingPreviousPartial =
@@ -535,6 +523,7 @@ export class Cline extends EventEmitter<ClineEvents> {
535523
this.updateClineMessage(lastMessage)
536524
} else {
537525
// this is a new partial message, so add it with partial state
526+
const sayTs = Date.now()
538527
await this.addToClineMessages({ ts: sayTs, type: "say", say: type, text, images, partial })
539528
}
540529
} else {
@@ -553,11 +542,13 @@ export class Cline extends EventEmitter<ClineEvents> {
553542
this.updateClineMessage(lastMessage)
554543
} else {
555544
// This is a new and complete message, so add it like normal.
545+
const sayTs = Date.now()
556546
await this.addToClineMessages({ ts: sayTs, type: "say", say: type, text, images })
557547
}
558548
}
559549
} else {
560550
// this is a new non-partial message, so add it like normal
551+
const sayTs = Date.now()
561552
await this.addToClineMessages({ ts: sayTs, type: "say", say: type, text, images, checkpoint })
562553
}
563554
}
@@ -1221,7 +1212,21 @@ export class Cline extends EventEmitter<ClineEvents> {
12211212
}
12221213

12231214
const baseDelay = requestDelaySeconds || 5
1224-
const exponentialDelay = Math.ceil(baseDelay * Math.pow(2, retryAttempt))
1215+
let exponentialDelay = Math.ceil(baseDelay * Math.pow(2, retryAttempt))
1216+
1217+
// If the error is a 429, and the error details contain a retry delay, use that delay instead of exponential backoff
1218+
if (error.status === 429) {
1219+
const geminiRetryDetails = error.errorDetails?.find(
1220+
(detail: any) => detail["@type"] === "type.googleapis.com/google.rpc.RetryInfo",
1221+
)
1222+
if (geminiRetryDetails) {
1223+
const match = geminiRetryDetails?.retryDelay?.match(/^(\d+)s$/)
1224+
if (match) {
1225+
exponentialDelay = Number(match[1]) + 1
1226+
}
1227+
}
1228+
}
1229+
12251230
// Wait for the greater of the exponential delay or the rate limit delay
12261231
const finalDelay = Math.max(exponentialDelay, rateLimitDelay)
12271232

@@ -2394,16 +2399,14 @@ export class Cline extends EventEmitter<ClineEvents> {
23942399
}
23952400
})
23962401

2397-
service.on("checkpoint", ({ isFirst, fromHash: from, toHash: to, startTime }) => {
2402+
service.on("checkpoint", ({ isFirst, fromHash: from, toHash: to }) => {
23982403
try {
23992404
this.providerRef.deref()?.postMessageToWebview({ type: "currentCheckpointUpdated", text: to })
24002405

2401-
this.say("checkpoint_saved", to, undefined, undefined, { isFirst, from, to, startTime }).catch(
2402-
(err) => {
2403-
log("[Cline#initializeCheckpoints] caught unexpected error in say('checkpoint_saved')")
2404-
console.error(err)
2405-
},
2406-
)
2406+
this.say("checkpoint_saved", to, undefined, undefined, { isFirst, from, to }).catch((err) => {
2407+
log("[Cline#initializeCheckpoints] caught unexpected error in say('checkpoint_saved')")
2408+
console.error(err)
2409+
})
24072410
} catch (err) {
24082411
log(
24092412
"[Cline#initializeCheckpoints] caught unexpected error in on('checkpoint'), disabling checkpoints",

‎src/core/config/ProviderSettingsManager.ts‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,7 +2,7 @@ import { ExtensionContext } from "vscode"
22
import { z } from "zod"
33

44
import { providerSettingsSchema, ApiConfigMeta } from "../../schemas"
5-
import { Mode } from "../../shared/modes"
5+
import { Mode, modes } from "../../shared/modes"
66

77
const providerSettingsWithIdSchema = providerSettingsSchema.extend({ id: z.string().optional() })
88

@@ -18,10 +18,16 @@ export type ProviderProfiles = z.infer<typeof providerProfilesSchema>
1818

1919
export class ProviderSettingsManager {
2020
private static readonly SCOPE_PREFIX = "roo_cline_config_"
21+
private readonly defaultConfigId = this.generateId()
22+
23+
private readonly defaultModeApiConfigs: Record<string, string> = Object.fromEntries(
24+
modes.map((mode) => [mode.slug, this.defaultConfigId]),
25+
)
2126

2227
private readonly defaultProviderProfiles: ProviderProfiles = {
2328
currentApiConfigName: "default",
24-
apiConfigs: { default: { id: this.generateId() } },
29+
apiConfigs: { default: { id: this.defaultConfigId } },
30+
modeApiConfigs: this.defaultModeApiConfigs,
2531
}
2632

2733
private readonly context: ExtensionContext

‎src/core/tools/__tests__/executeCommandTool.test.ts‎

Lines changed: 269 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,269 @@
1+
// npx jest src/core/tools/__tests__/executeCommandTool.test.ts
2+
3+
import { describe, expect, it, jest, beforeEach } from "@jest/globals"
4+
import { executeCommandTool } from "../executeCommandTool"
5+
import { Cline } from "../../Cline"
6+
import { ToolUse } from "../../assistant-message"
7+
import { formatResponse } from "../../prompts/responses"
8+
import { AskApproval, HandleError, PushToolResult, RemoveClosingTag } from "../types"
9+
import { ClineAsk } from "../../../schemas"
10+
11+
// Mock dependencies
12+
jest.mock("../../Cline")
13+
jest.mock("../../prompts/responses")
14+
15+
describe("executeCommandTool", () => {
16+
// Setup common test variables
17+
let mockCline: jest.Mocked<Partial<Cline>> & { consecutiveMistakeCount: number; didRejectTool: boolean }
18+
let mockAskApproval: jest.Mock
19+
let mockHandleError: jest.Mock
20+
let mockPushToolResult: jest.Mock
21+
let mockRemoveClosingTag: jest.Mock
22+
let mockToolUse: ToolUse
23+
24+
beforeEach(() => {
25+
// Reset mocks
26+
jest.clearAllMocks()
27+
28+
// Create mock implementations with eslint directives to handle the type issues
29+
mockCline = {
30+
// @ts-expect-error - Jest mock function type issues
31+
ask: jest.fn().mockResolvedValue(undefined),
32+
// @ts-expect-error - Jest mock function type issues
33+
say: jest.fn().mockResolvedValue(undefined),
34+
// @ts-expect-error - Jest mock function type issues
35+
sayAndCreateMissingParamError: jest.fn().mockResolvedValue("Missing parameter error"),
36+
// @ts-expect-error - Jest mock function type issues
37+
executeCommandTool: jest.fn().mockResolvedValue([false, "Command executed"]),
38+
consecutiveMistakeCount: 0,
39+
didRejectTool: false,
40+
rooIgnoreController: {
41+
// @ts-expect-error - Jest mock function type issues
42+
validateCommand: jest.fn().mockReturnValue(null),
43+
},
44+
}
45+
46+
// @ts-expect-error - Jest mock function type issues
47+
mockAskApproval = jest.fn().mockResolvedValue(true)
48+
// @ts-expect-error - Jest mock function type issues
49+
mockHandleError = jest.fn().mockResolvedValue(undefined)
50+
mockPushToolResult = jest.fn()
51+
mockRemoveClosingTag = jest.fn().mockReturnValue("command")
52+
53+
// Create a mock tool use object
54+
mockToolUse = {
55+
type: "tool_use",
56+
name: "execute_command",
57+
params: {
58+
command: "echo test",
59+
},
60+
partial: false,
61+
}
62+
})
63+
64+
/**
65+
* Tests for HTML entity unescaping in commands
66+
* This verifies that HTML entities are properly converted to their actual characters
67+
* before the command is executed
68+
*/
69+
describe("HTML entity unescaping", () => {
70+
it("should unescape &lt; to < character in commands", async () => {
71+
// Setup
72+
mockToolUse.params.command = "echo &lt;test&gt;"
73+
74+
// Execute
75+
await executeCommandTool(
76+
mockCline as unknown as Cline,
77+
mockToolUse,
78+
mockAskApproval as unknown as AskApproval,
79+
mockHandleError as unknown as HandleError,
80+
mockPushToolResult as unknown as PushToolResult,
81+
mockRemoveClosingTag as unknown as RemoveClosingTag,
82+
)
83+
84+
// Verify
85+
expect(mockAskApproval).toHaveBeenCalledWith("command", "echo <test>")
86+
expect(mockCline.executeCommandTool).toHaveBeenCalledWith("echo <test>", undefined)
87+
})
88+
89+
it("should unescape &gt; to > character in commands", async () => {
90+
// Setup
91+
mockToolUse.params.command = "echo test &gt; output.txt"
92+
93+
// Execute
94+
await executeCommandTool(
95+
mockCline as unknown as Cline,
96+
mockToolUse,
97+
mockAskApproval as unknown as AskApproval,
98+
mockHandleError as unknown as HandleError,
99+
mockPushToolResult as unknown as PushToolResult,
100+
mockRemoveClosingTag as unknown as RemoveClosingTag,
101+
)
102+
103+
// Verify
104+
expect(mockAskApproval).toHaveBeenCalledWith("command", "echo test > output.txt")
105+
expect(mockCline.executeCommandTool).toHaveBeenCalledWith("echo test > output.txt", undefined)
106+
})
107+
108+
it("should unescape &amp; to & character in commands", async () => {
109+
// Setup
110+
mockToolUse.params.command = "echo foo &amp;&amp; echo bar"
111+
112+
// Execute
113+
await executeCommandTool(
114+
mockCline as unknown as Cline,
115+
mockToolUse,
116+
mockAskApproval as unknown as AskApproval,
117+
mockHandleError as unknown as HandleError,
118+
mockPushToolResult as unknown as PushToolResult,
119+
mockRemoveClosingTag as unknown as RemoveClosingTag,
120+
)
121+
122+
// Verify
123+
expect(mockAskApproval).toHaveBeenCalledWith("command", "echo foo && echo bar")
124+
expect(mockCline.executeCommandTool).toHaveBeenCalledWith("echo foo && echo bar", undefined)
125+
})
126+
127+
it("should handle multiple mixed HTML entities in commands", async () => {
128+
// Setup
129+
mockToolUse.params.command = "grep -E 'pattern' &lt;file.txt &gt;output.txt 2&gt;&amp;1"
130+
131+
// Execute
132+
await executeCommandTool(
133+
mockCline as unknown as Cline,
134+
mockToolUse,
135+
mockAskApproval as unknown as AskApproval,
136+
mockHandleError as unknown as HandleError,
137+
mockPushToolResult as unknown as PushToolResult,
138+
mockRemoveClosingTag as unknown as RemoveClosingTag,
139+
)
140+
141+
// Verify
142+
const expectedCommand = "grep -E 'pattern' <file.txt >output.txt 2>&1"
143+
expect(mockAskApproval).toHaveBeenCalledWith("command", expectedCommand)
144+
expect(mockCline.executeCommandTool).toHaveBeenCalledWith(expectedCommand, undefined)
145+
})
146+
})
147+
148+
// Other functionality tests
149+
describe("Basic functionality", () => {
150+
it("should execute a command normally without HTML entities", async () => {
151+
// Setup
152+
mockToolUse.params.command = "echo test"
153+
154+
// Execute
155+
await executeCommandTool(
156+
mockCline as unknown as Cline,
157+
mockToolUse,
158+
mockAskApproval as unknown as AskApproval,
159+
mockHandleError as unknown as HandleError,
160+
mockPushToolResult as unknown as PushToolResult,
161+
mockRemoveClosingTag as unknown as RemoveClosingTag,
162+
)
163+
164+
// Verify
165+
expect(mockAskApproval).toHaveBeenCalledWith("command", "echo test")
166+
expect(mockCline.executeCommandTool).toHaveBeenCalledWith("echo test", undefined)
167+
expect(mockPushToolResult).toHaveBeenCalledWith("Command executed")
168+
})
169+
170+
it("should pass along custom working directory if provided", async () => {
171+
// Setup
172+
mockToolUse.params.command = "echo test"
173+
mockToolUse.params.cwd = "/custom/path"
174+
175+
// Execute
176+
await executeCommandTool(
177+
mockCline as unknown as Cline,
178+
mockToolUse,
179+
mockAskApproval as unknown as AskApproval,
180+
mockHandleError as unknown as HandleError,
181+
mockPushToolResult as unknown as PushToolResult,
182+
mockRemoveClosingTag as unknown as RemoveClosingTag,
183+
)
184+
185+
// Verify
186+
expect(mockCline.executeCommandTool).toHaveBeenCalledWith("echo test", "/custom/path")
187+
})
188+
})
189+
190+
describe("Error handling", () => {
191+
it("should handle missing command parameter", async () => {
192+
// Setup
193+
mockToolUse.params.command = undefined
194+
195+
// Execute
196+
await executeCommandTool(
197+
mockCline as unknown as Cline,
198+
mockToolUse,
199+
mockAskApproval as unknown as AskApproval,
200+
mockHandleError as unknown as HandleError,
201+
mockPushToolResult as unknown as PushToolResult,
202+
mockRemoveClosingTag as unknown as RemoveClosingTag,
203+
)
204+
205+
// Verify
206+
expect(mockCline.consecutiveMistakeCount).toBe(1)
207+
expect(mockCline.sayAndCreateMissingParamError).toHaveBeenCalledWith("execute_command", "command")
208+
expect(mockPushToolResult).toHaveBeenCalledWith("Missing parameter error")
209+
expect(mockAskApproval).not.toHaveBeenCalled()
210+
expect(mockCline.executeCommandTool).not.toHaveBeenCalled()
211+
})
212+
213+
it("should handle command rejection", async () => {
214+
// Setup
215+
mockToolUse.params.command = "echo test"
216+
// @ts-expect-error - Jest mock function type issues
217+
mockAskApproval.mockResolvedValue(false)
218+
219+
// Execute
220+
await executeCommandTool(
221+
mockCline as unknown as Cline,
222+
mockToolUse,
223+
mockAskApproval as unknown as AskApproval,
224+
mockHandleError as unknown as HandleError,
225+
mockPushToolResult as unknown as PushToolResult,
226+
mockRemoveClosingTag as unknown as RemoveClosingTag,
227+
)
228+
229+
// Verify
230+
expect(mockAskApproval).toHaveBeenCalledWith("command", "echo test")
231+
expect(mockCline.executeCommandTool).not.toHaveBeenCalled()
232+
expect(mockPushToolResult).not.toHaveBeenCalled()
233+
})
234+
235+
it("should handle rooignore validation failures", async () => {
236+
// Setup
237+
mockToolUse.params.command = "cat .env"
238+
// Override the validateCommand mock to return a filename
239+
const validateCommandMock = jest.fn().mockReturnValue(".env")
240+
mockCline.rooIgnoreController = {
241+
// @ts-expect-error - Jest mock function type issues
242+
validateCommand: validateCommandMock,
243+
}
244+
245+
const mockRooIgnoreError = "RooIgnore error"
246+
;(formatResponse.rooIgnoreError as jest.Mock).mockReturnValue(mockRooIgnoreError)
247+
;(formatResponse.toolError as jest.Mock).mockReturnValue("Tool error")
248+
249+
// Execute
250+
await executeCommandTool(
251+
mockCline as unknown as Cline,
252+
mockToolUse,
253+
mockAskApproval as unknown as AskApproval,
254+
mockHandleError as unknown as HandleError,
255+
mockPushToolResult as unknown as PushToolResult,
256+
mockRemoveClosingTag as unknown as RemoveClosingTag,
257+
)
258+
259+
// Verify
260+
expect(validateCommandMock).toHaveBeenCalledWith("cat .env")
261+
expect(mockCline.say).toHaveBeenCalledWith("rooignore_error", ".env")
262+
expect(formatResponse.rooIgnoreError).toHaveBeenCalledWith(".env")
263+
expect(formatResponse.toolError).toHaveBeenCalledWith(mockRooIgnoreError)
264+
expect(mockPushToolResult).toHaveBeenCalled()
265+
expect(mockAskApproval).not.toHaveBeenCalled()
266+
expect(mockCline.executeCommandTool).not.toHaveBeenCalled()
267+
})
268+
})
269+
})

0 commit comments

Comments
 (0)