Skip to content
Draft
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
2 changes: 2 additions & 0 deletions packages/types/src/global-settings.ts
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,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 @@ -264,6 +265,7 @@ export const EVALS_SETTINGS: RooCodeSettings = {
diagnosticsEnabled: true,

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

enableCheckpoints: false,
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
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 @@ -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