Skip to content
Closed
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
5 changes: 5 additions & 0 deletions .changeset/green-rings-notice.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"roo-cline": patch
---

Adds file based editing as experimental feature.
3 changes: 2 additions & 1 deletion packages/types/src/experiment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,7 +6,7 @@ import type { Keys, Equals, AssertEqual } from "./type-fu.js"
* ExperimentId
*/

export const experimentIds = ["powerSteering", "multiFileApplyDiff"] as const
export const experimentIds = ["powerSteering", "multiFileApplyDiff", "fileBasedEditing"] as const

export const experimentIdsSchema = z.enum(experimentIds)

Expand All @@ -19,6 +19,7 @@ export type ExperimentId = z.infer<typeof experimentIdsSchema>
export const experimentsSchema = z.object({
powerSteering: z.boolean().optional(),
multiFileApplyDiff: z.boolean().optional(),
fileBasedEditing: z.boolean().optional(),
})

export type Experiments = z.infer<typeof experimentsSchema>
Expand Down
48 changes: 38 additions & 10 deletions src/core/task/Task.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,7 @@ import {
TodoItem,
getApiProtocol,
getModelId,
Experiments,
} from "@roo-code/types"
import { TelemetryService } from "@roo-code/telemetry"
import { CloudService } from "@roo-code/cloud"
Expand Down Expand Up @@ -54,7 +55,6 @@ import { McpServerManager } from "../../services/mcp/McpServerManager"
import { RepoPerTaskCheckpointService } from "../../services/checkpoints"

// integrations
import { DiffViewProvider } from "../../integrations/editor/DiffViewProvider"
import { findToolName, formatContentBlockToMarkdown } from "../../integrations/misc/export-markdown"
import { RooTerminalProcess } from "../../integrations/terminal/types"
import { TerminalRegistry } from "../../integrations/terminal/TerminalRegistry"
Expand Down Expand Up @@ -92,6 +92,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 @@ -120,7 +122,7 @@ export type TaskOptions = {
task?: string
images?: string[]
historyItem?: HistoryItem
experiments?: Record<string, boolean>
experiments?: Experiments
startTask?: boolean
rootTask?: Task
parentTask?: Task
Expand Down Expand Up @@ -173,7 +175,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 @@ -261,7 +263,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)
this.editingProvider = EditingProviderFactory.createEditingProvider(this.cwd, {}, this)
this.enableCheckpoints = enableCheckpoints

this.rootTask = rootTask
Expand All @@ -274,14 +276,31 @@ export class Task extends EventEmitter<ClineEvents> {
TelemetryService.instance.captureTaskCreated(this.taskId)
}

// Check experiment asynchronously and editingprovider if needed
provider.getState().then((state) => {
const isFileBasedEditingEnabled = experiments?.isEnabled(
state.experiments ?? {},
EXPERIMENT_IDS.FILE_BASED_EDITING,
)

if (isFileBasedEditingEnabled) {
this.editingProvider = EditingProviderFactory.resetAndCreateNewEditingProvider(
this.cwd,
this.editingProvider,
state.experiments ?? {},
this,
)
}
})

// Only set up diff strategy if diff is enabled
if (this.diffEnabled) {
// Default to old strategy, will be updated if experiment is enabled
this.diffStrategy = new MultiSearchReplaceDiffStrategy(this.fuzzyMatchThreshold)

// Check experiment asynchronously and update strategy if needed
provider.getState().then((state) => {
const isMultiFileApplyDiffEnabled = experiments.isEnabled(
const isMultiFileApplyDiffEnabled = experiments?.isEnabled(
state.experiments ?? {},
EXPERIMENT_IDS.MULTI_FILE_APPLY_DIFF,
)
Expand Down Expand Up @@ -763,6 +782,8 @@ export class Task extends EventEmitter<ClineEvents> {

public async resumePausedTask(lastMessage: string) {
// Release this Cline instance from paused state.
const context = await this.providerRef.deref()?.getState()
this.editingProvider = EditingProviderFactory.createEditingProvider(this.cwd, context?.experiments ?? {}, this)
this.isPaused = false
this.emit("taskUnpaused")

Expand Down Expand Up @@ -1067,8 +1088,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 @@ -1161,6 +1182,13 @@ export class Task extends EventEmitter<ClineEvents> {
if (this.abort) {
throw new Error(`[RooCode#recursivelyMakeRooRequests] task ${this.taskId}.${this.instanceId} aborted`)
}
const context = await this.providerRef.deref()?.getState()
this.editingProvider = EditingProviderFactory.resetAndCreateNewEditingProvider(
this.cwd,
this.editingProvider,
context?.experiments ?? {},
this,
)

if (this.consecutiveMistakeLimit > 0 && this.consecutiveMistakeCount >= this.consecutiveMistakeLimit) {
const { response, text, images } = await this.ask(
Expand Down Expand Up @@ -1303,8 +1331,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 @@ -1356,7 +1384,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
1 change: 1 addition & 0 deletions src/core/task/__tests__/Task.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1386,6 +1386,7 @@ describe("Cline", () => {
})

it("should not create diff strategy when enableDiff is false", async () => {
mockProvider.getState.mockResolvedValue({})
const task = new Task({
provider: mockProvider,
apiConfiguration: mockApiConfig,
Expand Down
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 @@ -180,9 +180,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 @@ -196,9 +196,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 @@ -208,9 +208,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 @@ -227,8 +227,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 @@ -247,7 +247,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 @@ -256,18 +256,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 @@ -279,13 +279,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 @@ -313,7 +313,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 @@ -322,10 +322,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 @@ -350,21 +350,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 @@ -374,19 +374,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 @@ -399,21 +399,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()
})
})
})
Loading
Loading