Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions packages/types/src/global-settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,6 +107,7 @@ export const globalSettingsSchema = z.object({

rateLimitSeconds: z.number().optional(),
diffEnabled: z.boolean().optional(),
fileBasedEditing: z.boolean().optional(),
fuzzyMatchThreshold: z.number().optional(),
experiments: experimentsSchema.optional(),

Expand Down Expand Up @@ -250,6 +251,7 @@ export const EVALS_SETTINGS: RooCodeSettings = {
diagnosticsEnabled: true,

diffEnabled: true,
fileBasedEditing: false,
fuzzyMatchThreshold: 1,

enableCheckpoints: false,
Expand Down
18 changes: 11 additions & 7 deletions src/core/task/Task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,8 @@ import { ApiMessage } from "../task-persistence/apiMessages"
import { getMessagesSinceLastSummary, summarizeConversation } from "../condense"
import { maybeRemoveImageBlocks } from "../../api/transform/image-cleaning"
import { restoreTodoListForTask } from "../tools/updateTodoListTool"
import { IEditingProvider } from "../../integrations/editor/IEditingProvider"
import { EditingProviderFactory } from "../../integrations/editor/EditingProviderFactory"

// Constants
const MAX_EXPONENTIAL_BACKOFF_SECONDS = 600 // 10 minutes
Expand Down Expand Up @@ -172,7 +174,7 @@ export class Task extends EventEmitter<ClineEvents> {
browserSession: BrowserSession

// Editing
diffViewProvider: DiffViewProvider
editingProvider: IEditingProvider
diffStrategy?: DiffStrategy
diffEnabled: boolean = false
fuzzyMatchThreshold: number
Expand Down Expand Up @@ -260,7 +262,7 @@ export class Task extends EventEmitter<ClineEvents> {
this.consecutiveMistakeLimit = consecutiveMistakeLimit ?? DEFAULT_CONSECUTIVE_MISTAKE_LIMIT
this.providerRef = new WeakRef(provider)
this.globalStoragePath = provider.context.globalStorageUri.fsPath
this.diffViewProvider = new DiffViewProvider(this.cwd)
this.editingProvider = EditingProviderFactory.createEditingProvider(this.cwd)
this.enableCheckpoints = enableCheckpoints

this.rootTask = rootTask
Expand Down Expand Up @@ -762,6 +764,7 @@ export class Task extends EventEmitter<ClineEvents> {

public async resumePausedTask(lastMessage: string) {
// Release this Cline instance from paused state.
this.editingProvider = EditingProviderFactory.createEditingProvider(this.cwd)
this.isPaused = false
this.emit("taskUnpaused")

Expand Down Expand Up @@ -1066,8 +1069,8 @@ export class Task extends EventEmitter<ClineEvents> {

try {
// If we're not streaming then `abortStream` won't be called
if (this.isStreaming && this.diffViewProvider.isEditing) {
this.diffViewProvider.revertChanges().catch(console.error)
if (this.isStreaming && this.editingProvider.isEditing) {
this.editingProvider.revertChanges().catch(console.error)
}
} catch (error) {
console.error("Error reverting diff changes:", error)
Expand Down Expand Up @@ -1160,6 +1163,7 @@ export class Task extends EventEmitter<ClineEvents> {
if (this.abort) {
throw new Error(`[RooCode#recursivelyMakeRooRequests] task ${this.taskId}.${this.instanceId} aborted`)
}
this.editingProvider = EditingProviderFactory.resetAndCreateNewEditingProvider(this.cwd, this.editingProvider)

if (this.consecutiveMistakeLimit > 0 && this.consecutiveMistakeCount >= this.consecutiveMistakeLimit) {
const { response, text, images } = await this.ask(
Expand Down Expand Up @@ -1296,8 +1300,8 @@ export class Task extends EventEmitter<ClineEvents> {
}

const abortStream = async (cancelReason: ClineApiReqCancelReason, streamingFailedMessage?: string) => {
if (this.diffViewProvider.isEditing) {
await this.diffViewProvider.revertChanges() // closes diff view
if (this.editingProvider.isEditing) {
await this.editingProvider.revertChanges() // closes diff view
}

// if last message is a partial we need to update and save it
Expand Down Expand Up @@ -1349,7 +1353,7 @@ export class Task extends EventEmitter<ClineEvents> {
this.presentAssistantMessageLocked = false
this.presentAssistantMessageHasPendingUpdates = false

await this.diffViewProvider.reset()
await this.editingProvider.reset()

// Yields only if the first chunk is successful, otherwise will
// allow the user to retry the request (most likely due to rate
Expand Down
2 changes: 1 addition & 1 deletion src/core/tools/__tests__/applyDiffTool.experiment.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,7 @@ describe("applyDiffTool experiment routing", () => {
applyDiff: vi.fn(),
getProgressStatus: vi.fn(),
},
diffViewProvider: {
editingProvider: {
reset: vi.fn(),
},
api: {
Expand Down
24 changes: 12 additions & 12 deletions src/core/tools/__tests__/insertContentTool.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -82,7 +82,7 @@ describe("insertContentTool", () => {
rooIgnoreController: {
validateAccess: vi.fn().mockReturnValue(true),
},
diffViewProvider: {
editingProvider: {
editType: undefined,
isEditing: false,
originalContent: "",
Expand Down Expand Up @@ -179,9 +179,9 @@ describe("insertContentTool", () => {
const calledPath = mockedFileExistsAtPath.mock.calls[0][0]
expect(toPosix(calledPath)).toContain(testFilePath)
expect(mockedFsReadFile).not.toHaveBeenCalled() // Should not read if file doesn't exist
expect(mockCline.diffViewProvider.update).toHaveBeenCalledWith(contentToInsert, true)
expect(mockCline.diffViewProvider.editType).toBe("create")
expect(mockCline.diffViewProvider.pushToolWriteResult).toHaveBeenCalledWith(mockCline, mockCline.cwd, true)
expect(mockCline.editingProvider.update).toHaveBeenCalledWith(contentToInsert, true)
expect(mockCline.editingProvider.editType).toBe("create")
expect(mockCline.editingProvider.pushToolWriteResult).toHaveBeenCalledWith(mockCline, mockCline.cwd, true)
})

it("creates a new file and inserts content at line 1 (beginning)", async () => {
Expand All @@ -195,9 +195,9 @@ describe("insertContentTool", () => {
const calledPath = mockedFileExistsAtPath.mock.calls[0][0]
expect(toPosix(calledPath)).toContain(testFilePath)
expect(mockedFsReadFile).not.toHaveBeenCalled()
expect(mockCline.diffViewProvider.update).toHaveBeenCalledWith(contentToInsert, true)
expect(mockCline.diffViewProvider.editType).toBe("create")
expect(mockCline.diffViewProvider.pushToolWriteResult).toHaveBeenCalledWith(mockCline, mockCline.cwd, true)
expect(mockCline.editingProvider.update).toHaveBeenCalledWith(contentToInsert, true)
expect(mockCline.editingProvider.editType).toBe("create")
expect(mockCline.editingProvider.pushToolWriteResult).toHaveBeenCalledWith(mockCline, mockCline.cwd, true)
})

it("creates an empty new file if content is empty string", async () => {
Expand All @@ -207,9 +207,9 @@ describe("insertContentTool", () => {
const calledPath = mockedFileExistsAtPath.mock.calls[0][0]
expect(toPosix(calledPath)).toContain(testFilePath)
expect(mockedFsReadFile).not.toHaveBeenCalled()
expect(mockCline.diffViewProvider.update).toHaveBeenCalledWith("", true)
expect(mockCline.diffViewProvider.editType).toBe("create")
expect(mockCline.diffViewProvider.pushToolWriteResult).toHaveBeenCalledWith(mockCline, mockCline.cwd, true)
expect(mockCline.editingProvider.update).toHaveBeenCalledWith("", true)
expect(mockCline.editingProvider.editType).toBe("create")
expect(mockCline.editingProvider.pushToolWriteResult).toHaveBeenCalledWith(mockCline, mockCline.cwd, true)
})

it("returns an error when inserting content at an arbitrary line number into a new file", async () => {
Expand All @@ -226,8 +226,8 @@ describe("insertContentTool", () => {
expect(mockCline.consecutiveMistakeCount).toBe(1)
expect(mockCline.recordToolError).toHaveBeenCalledWith("insert_content")
expect(mockCline.say).toHaveBeenCalledWith("error", expect.stringContaining("non-existent file"))
expect(mockCline.diffViewProvider.update).not.toHaveBeenCalled()
expect(mockCline.diffViewProvider.pushToolWriteResult).not.toHaveBeenCalled()
expect(mockCline.editingProvider.update).not.toHaveBeenCalled()
expect(mockCline.editingProvider.pushToolWriteResult).not.toHaveBeenCalled()
})
})
})
48 changes: 24 additions & 24 deletions src/core/tools/__tests__/writeToFileTool.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,7 +143,7 @@ describe("writeToFileTool", () => {
mockCline.rooIgnoreController = {
validateAccess: vi.fn().mockReturnValue(true),
}
mockCline.diffViewProvider = {
mockCline.editingProvider = {
editType: undefined,
isEditing: false,
originalContent: "",
Expand Down Expand Up @@ -246,7 +246,7 @@ describe("writeToFileTool", () => {
await executeWriteFileTool({}, { accessAllowed: true })

expect(mockCline.rooIgnoreController.validateAccess).toHaveBeenCalledWith(testFilePath)
expect(mockCline.diffViewProvider.open).toHaveBeenCalledWith(testFilePath)
expect(mockCline.editingProvider.open).toHaveBeenCalledWith(testFilePath)
})
})

Expand All @@ -255,18 +255,18 @@ describe("writeToFileTool", () => {
await executeWriteFileTool({}, { fileExists: true })

expect(mockedFileExistsAtPath).toHaveBeenCalledWith(absoluteFilePath)
expect(mockCline.diffViewProvider.editType).toBe("modify")
expect(mockCline.editingProvider.editType).toBe("modify")
})

it.skipIf(process.platform === "win32")("detects new file and sets editType to create", async () => {
await executeWriteFileTool({}, { fileExists: false })

expect(mockedFileExistsAtPath).toHaveBeenCalledWith(absoluteFilePath)
expect(mockCline.diffViewProvider.editType).toBe("create")
expect(mockCline.editingProvider.editType).toBe("create")
})

it("uses cached editType without filesystem check", async () => {
mockCline.diffViewProvider.editType = "modify"
mockCline.editingProvider.editType = "modify"

await executeWriteFileTool({})

Expand All @@ -278,13 +278,13 @@ describe("writeToFileTool", () => {
it("removes markdown code block markers from content", async () => {
await executeWriteFileTool({ content: testContentWithMarkdown })

expect(mockCline.diffViewProvider.update).toHaveBeenCalledWith("Line 1\nLine 2", true)
expect(mockCline.editingProvider.update).toHaveBeenCalledWith("Line 1\nLine 2", true)
})

it("passes through empty content unchanged", async () => {
await executeWriteFileTool({ content: "" })

expect(mockCline.diffViewProvider.update).toHaveBeenCalledWith("", true)
expect(mockCline.editingProvider.update).toHaveBeenCalledWith("", true)
})

it("unescapes HTML entities for non-Claude models", async () => {
Expand Down Expand Up @@ -312,7 +312,7 @@ describe("writeToFileTool", () => {

expect(mockedEveryLineHasLineNumbers).toHaveBeenCalledWith(contentWithLineNumbers)
expect(mockedStripLineNumbers).toHaveBeenCalledWith(contentWithLineNumbers)
expect(mockCline.diffViewProvider.update).toHaveBeenCalledWith("line one\nline two", true)
expect(mockCline.editingProvider.update).toHaveBeenCalledWith("line one\nline two", true)
})
})

Expand All @@ -321,10 +321,10 @@ describe("writeToFileTool", () => {
await executeWriteFileTool({}, { fileExists: false })

expect(mockCline.consecutiveMistakeCount).toBe(0)
expect(mockCline.diffViewProvider.open).toHaveBeenCalledWith(testFilePath)
expect(mockCline.diffViewProvider.update).toHaveBeenCalledWith(testContent, true)
expect(mockCline.editingProvider.open).toHaveBeenCalledWith(testFilePath)
expect(mockCline.editingProvider.update).toHaveBeenCalledWith(testContent, true)
expect(mockAskApproval).toHaveBeenCalled()
expect(mockCline.diffViewProvider.saveChanges).toHaveBeenCalled()
expect(mockCline.editingProvider.saveChanges).toHaveBeenCalled()
expect(mockCline.fileContextTracker.trackFileContext).toHaveBeenCalledWith(testFilePath, "roo_edited")
expect(mockCline.didEditFile).toBe(true)
})
Expand All @@ -349,21 +349,21 @@ describe("writeToFileTool", () => {
it("returns early when path is missing in partial block", async () => {
await executeWriteFileTool({ path: undefined }, { isPartial: true })

expect(mockCline.diffViewProvider.open).not.toHaveBeenCalled()
expect(mockCline.editingProvider.open).not.toHaveBeenCalled()
})

it("returns early when content is undefined in partial block", async () => {
await executeWriteFileTool({ content: undefined }, { isPartial: true })

expect(mockCline.diffViewProvider.open).not.toHaveBeenCalled()
expect(mockCline.editingProvider.open).not.toHaveBeenCalled()
})

it("streams content updates during partial execution", async () => {
await executeWriteFileTool({}, { isPartial: true })

expect(mockCline.ask).toHaveBeenCalled()
expect(mockCline.diffViewProvider.open).toHaveBeenCalledWith(testFilePath)
expect(mockCline.diffViewProvider.update).toHaveBeenCalledWith(testContent, false)
expect(mockCline.editingProvider.open).toHaveBeenCalledWith(testFilePath)
expect(mockCline.editingProvider.update).toHaveBeenCalledWith(testContent, false)
})
})

Expand All @@ -373,19 +373,19 @@ describe("writeToFileTool", () => {

await executeWriteFileTool({})

expect(mockCline.diffViewProvider.revertChanges).toHaveBeenCalled()
expect(mockCline.diffViewProvider.saveChanges).not.toHaveBeenCalled()
expect(mockCline.editingProvider.revertChanges).toHaveBeenCalled()
expect(mockCline.editingProvider.saveChanges).not.toHaveBeenCalled()
})

it("reports user edits with diff feedback", async () => {
const userEditsValue = "- old line\n+ new line"
mockCline.diffViewProvider.saveChanges.mockResolvedValue({
mockCline.editingProvider.saveChanges.mockResolvedValue({
newProblemsMessage: " with warnings",
userEdits: userEditsValue,
finalContent: "modified content",
})
// Set the userEdits property on the diffViewProvider mock to simulate user edits
mockCline.diffViewProvider.userEdits = userEditsValue
// Set the userEdits property on the editingProvider mock to simulate user edits
mockCline.editingProvider.userEdits = userEditsValue

await executeWriteFileTool({}, { fileExists: true })

Expand All @@ -398,21 +398,21 @@ describe("writeToFileTool", () => {

describe("error handling", () => {
it("handles general file operation errors", async () => {
mockCline.diffViewProvider.open.mockRejectedValue(new Error("General error"))
mockCline.editingProvider.open.mockRejectedValue(new Error("General error"))

await executeWriteFileTool({})

expect(mockHandleError).toHaveBeenCalledWith("writing file", expect.any(Error))
expect(mockCline.diffViewProvider.reset).toHaveBeenCalled()
expect(mockCline.editingProvider.reset).toHaveBeenCalled()
})

it("handles partial streaming errors", async () => {
mockCline.diffViewProvider.open.mockRejectedValue(new Error("Open failed"))
mockCline.editingProvider.open.mockRejectedValue(new Error("Open failed"))

await executeWriteFileTool({}, { isPartial: true })

expect(mockHandleError).toHaveBeenCalledWith("writing file", expect.any(Error))
expect(mockCline.diffViewProvider.reset).toHaveBeenCalled()
expect(mockCline.editingProvider.reset).toHaveBeenCalled()
})
})
})
18 changes: 9 additions & 9 deletions src/core/tools/applyDiffTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -143,10 +143,10 @@ export async function applyDiffToolLegacy(
cline.consecutiveMistakeCountForApplyDiff.delete(relPath)

// Show diff view before asking for approval
cline.diffViewProvider.editType = "modify"
await cline.diffViewProvider.open(relPath)
await cline.diffViewProvider.update(diffResult.content, true)
cline.diffViewProvider.scrollToFirstDiff()
cline.editingProvider.editType = "modify"
await cline.editingProvider.open(relPath)
await cline.editingProvider.update(diffResult.content, true)
cline.editingProvider.scrollToFirstDiff()

// Check if file is write-protected
const isWriteProtected = cline.rooProtectedController?.isWriteProtected(relPath) || false
Expand All @@ -166,7 +166,7 @@ export async function applyDiffToolLegacy(
const didApprove = await askApproval("tool", completeMessage, toolProgressStatus, isWriteProtected)

if (!didApprove) {
await cline.diffViewProvider.revertChanges() // Cline likely handles closing the diff view
await cline.editingProvider.revertChanges() // Cline likely handles closing the diff view
return
}

Expand All @@ -175,7 +175,7 @@ export async function applyDiffToolLegacy(
const state = await provider?.getState()
const diagnosticsEnabled = state?.diagnosticsEnabled ?? true
const writeDelayMs = state?.writeDelayMs ?? DEFAULT_WRITE_DELAY_MS
await cline.diffViewProvider.saveChanges(diagnosticsEnabled, writeDelayMs)
await cline.editingProvider.saveChanges(diagnosticsEnabled, writeDelayMs)

// Track file edit operation
if (relPath) {
Expand All @@ -191,21 +191,21 @@ export async function applyDiffToolLegacy(
}

// Get the formatted response message
const message = await cline.diffViewProvider.pushToolWriteResult(cline, cline.cwd, !fileExists)
const message = await cline.editingProvider.pushToolWriteResult(cline, cline.cwd, !fileExists)

if (partFailHint) {
pushToolResult(partFailHint + message)
} else {
pushToolResult(message)
}

await cline.diffViewProvider.reset()
await cline.editingProvider.reset()

return
}
} catch (error) {
await handleError("applying diff", error)
await cline.diffViewProvider.reset()
await cline.editingProvider.reset()
return
}
}
Loading
Loading