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
20 changes: 20 additions & 0 deletions src/core/tools/__tests__/insertContentTool.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,7 @@ import type { MockedFunction } from "vitest"
import { fileExistsAtPath } from "../../../utils/fs"
import { ToolUse, ToolResponse } from "../../../shared/tools"
import { insertContentTool } from "../insertContentTool"
import { experiments } from "../../../shared/experiments"

// Helper to normalize paths to POSIX format for cross-platform testing
const toPosix = (filePath: string) => filePath.replace(/\\/g, "/")
Expand All @@ -23,6 +24,22 @@ vi.mock("../../../utils/fs", () => ({
fileExistsAtPath: vi.fn().mockResolvedValue(false),
}))

vi.mock("../../../shared/experiments", () => ({
experiments: {
isEnabled: vi.fn().mockReturnValue(false),
},
EXPERIMENT_IDS: {
PREVENT_FOCUS_DISRUPTION: "preventFocusDisruption",
},
}))

vi.mock("../../../utils/activity-detector", () => ({
getActivityDetector: vi.fn().mockReturnValue({
isUserActive: vi.fn().mockReturnValue(false),
waitForInactivity: vi.fn().mockResolvedValue(true),
}),
}))

vi.mock("../../prompts/responses", () => ({
formatResponse: {
toolError: vi.fn((msg) => `Error: ${msg}`),
Expand Down Expand Up @@ -64,6 +81,9 @@ describe("insertContentTool", () => {
beforeEach(() => {
vi.clearAllMocks()

// Mock experiments to be disabled by default for tests
vi.mocked(experiments).isEnabled.mockReturnValue(false)

mockedFileExistsAtPath.mockResolvedValue(true) // Assume file exists by default for insert
mockedFsReadFile.mockResolvedValue("") // Default empty file content

Expand Down
13 changes: 13 additions & 0 deletions src/core/tools/__tests__/writeToFileTool.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@ import { unescapeHtmlEntities } from "../../../utils/text-normalization"
import { everyLineHasLineNumbers, stripLineNumbers } from "../../../integrations/misc/extract-text"
import { ToolUse, ToolResponse } from "../../../shared/tools"
import { writeToFileTool } from "../writeToFileTool"
import { experiments } from "../../../shared/experiments"

vi.mock("path", async () => {
const originalPath = await vi.importActual("path")
Expand Down Expand Up @@ -92,6 +93,15 @@ vi.mock("../../ignore/RooIgnoreController", () => ({
},
}))

vi.mock("../../../shared/experiments", () => ({
EXPERIMENT_IDS: {
PREVENT_FOCUS_DISRUPTION: "preventFocusDisruption",
},
experiments: {
isEnabled: vi.fn().mockReturnValue(false), // Default to disabled for tests
},
}))

describe("writeToFileTool", () => {
// Test data
const testFilePath = "test/file.txt"
Expand Down Expand Up @@ -119,6 +129,9 @@ describe("writeToFileTool", () => {
beforeEach(() => {
vi.clearAllMocks()

// Mock experiments to be disabled by default for tests
vi.mocked(experiments.isEnabled).mockReturnValue(false)

mockedPathResolve.mockReturnValue(absoluteFilePath)
mockedFileExistsAtPath.mockResolvedValue(false)
mockedDetectCodeOmission.mockReturnValue(false)
Expand Down
27 changes: 27 additions & 0 deletions src/core/tools/applyDiffTool.ts
Original file line number Diff line number Diff line change
@@ -1,5 +1,6 @@
import path from "path"
import fs from "fs/promises"
import * as vscode from "vscode"

import { TelemetryService } from "@roo-code/telemetry"
import { DEFAULT_WRITE_DELAY_MS } from "@roo-code/types"
Expand All @@ -13,6 +14,7 @@ import { fileExistsAtPath } from "../../utils/fs"
import { RecordSource } from "../context-tracking/FileContextTrackerTypes"
import { unescapeHtmlEntities } from "../../utils/text-normalization"
import { EXPERIMENT_IDS, experiments } from "../../shared/experiments"
import { getActivityDetector } from "../../utils/activity-detector"

export async function applyDiffToolLegacy(
cline: Task,
Expand Down Expand Up @@ -154,6 +156,31 @@ export async function applyDiffToolLegacy(
const isWriteProtected = cline.rooProtectedController?.isWriteProtected(relPath) || false

if (isPreventFocusDisruptionEnabled) {
// Wait for user to become inactive before proceeding with file operations
const activityDetector = getActivityDetector()
if (activityDetector.isUserActive()) {
// Wait up to 10 seconds for user to become inactive
const becameInactive = await activityDetector.waitForInactivity(10000)

if (!becameInactive) {
// User is still active after timeout, ask for permission to proceed
const shouldProceed = await vscode.window.showWarningMessage(
"You appear to be actively editing. Would you like Roo Code to proceed with file changes anyway?",
"Proceed",
"Cancel",
)

if (shouldProceed !== "Proceed") {
pushToolResult(
formatResponse.toolError(
"File operation cancelled to avoid disrupting active editing.",
),
)
return
}
}
}

// Direct file write without diff view
const completeMessage = JSON.stringify({
...sharedMessageProps,
Expand Down
26 changes: 26 additions & 0 deletions src/core/tools/insertContentTool.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import delay from "delay"
import fs from "fs/promises"
import path from "path"
import * as vscode from "vscode"

import { getReadablePath } from "../../utils/path"
import { Task } from "../task/Task"
Expand All @@ -12,6 +13,7 @@ import { fileExistsAtPath } from "../../utils/fs"
import { insertGroups } from "../diff/insert-groups"
import { DEFAULT_WRITE_DELAY_MS } from "@roo-code/types"
import { EXPERIMENT_IDS, experiments } from "../../shared/experiments"
import { getActivityDetector } from "../../utils/activity-detector"

export async function insertContentTool(
cline: Task,
Expand Down Expand Up @@ -150,6 +152,30 @@ export async function insertContentTool(
await cline.diffViewProvider.open(relPath)
await cline.diffViewProvider.update(updatedContent, true)
cline.diffViewProvider.scrollToFirstDiff()
} else {
// Wait for user to become inactive before proceeding with file operations
const activityDetector = getActivityDetector()
if (activityDetector.isUserActive()) {
// Wait up to 10 seconds for user to become inactive
const becameInactive = await activityDetector.waitForInactivity(10000)

if (!becameInactive) {
// User is still active after timeout, ask for permission to proceed
const shouldProceed = await vscode.window.showWarningMessage(
"You appear to be actively editing. Would you like Roo Code to proceed with file changes anyway?",
"Proceed",
"Cancel",
)

if (shouldProceed !== "Proceed") {
pushToolResult(
formatResponse.toolError("File operation cancelled to avoid disrupting active editing."),
)
await cline.diffViewProvider.reset()
return
}
}
}
}

// Ask for approval (same for both flows)
Expand Down
26 changes: 26 additions & 0 deletions src/core/tools/searchAndReplaceTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@
import path from "path"
import fs from "fs/promises"
import delay from "delay"
import * as vscode from "vscode"

// Internal imports
import { Task } from "../task/Task"
Expand All @@ -13,6 +14,7 @@ import { fileExistsAtPath } from "../../utils/fs"
import { RecordSource } from "../context-tracking/FileContextTrackerTypes"
import { DEFAULT_WRITE_DELAY_MS } from "@roo-code/types"
import { EXPERIMENT_IDS, experiments } from "../../shared/experiments"
import { getActivityDetector } from "../../utils/activity-detector"

/**
* Tool for performing search and replace operations on files
Expand Down Expand Up @@ -221,6 +223,30 @@ export async function searchAndReplaceTool(
await cline.diffViewProvider.open(validRelPath)
await cline.diffViewProvider.update(newContent, true)
cline.diffViewProvider.scrollToFirstDiff()
} else {
// Wait for user to become inactive before proceeding with file operations
const activityDetector = getActivityDetector()
if (activityDetector.isUserActive()) {
// Wait up to 10 seconds for user to become inactive
const becameInactive = await activityDetector.waitForInactivity(10000)

if (!becameInactive) {
// User is still active after timeout, ask for permission to proceed
const shouldProceed = await vscode.window.showWarningMessage(
"You appear to be actively editing. Would you like Roo Code to proceed with file changes anyway?",
"Proceed",
"Cancel",
)

if (shouldProceed !== "Proceed") {
pushToolResult(
formatResponse.toolError("File operation cancelled to avoid disrupting active editing."),
)
await cline.diffViewProvider.reset()
return
}
}
}
}

const didApprove = await askApproval("tool", completeMessage, undefined, isWriteProtected)
Expand Down
30 changes: 30 additions & 0 deletions src/core/tools/writeToFileTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { detectCodeOmission } from "../../integrations/editor/detect-omission"
import { unescapeHtmlEntities } from "../../utils/text-normalization"
import { DEFAULT_WRITE_DELAY_MS } from "@roo-code/types"
import { EXPERIMENT_IDS, experiments } from "../../shared/experiments"
import { getActivityDetector } from "../../utils/activity-detector"

export async function writeToFileTool(
cline: Task,
Expand Down Expand Up @@ -172,6 +173,35 @@ export async function writeToFileTool(
)

if (isPreventFocusDisruptionEnabled) {
// Wait for user to become inactive before proceeding with file operations
const activityDetector = getActivityDetector()
if (activityDetector.isUserActive()) {
// Notify user that we're waiting for them to stop typing
await cline.say("text", "Waiting for you to finish typing before making file changes...")

// Wait up to 10 seconds for user to become inactive
const becameInactive = await activityDetector.waitForInactivity(10000)

if (!becameInactive) {
// User is still active after timeout, ask for permission to proceed
const shouldProceed = await vscode.window.showWarningMessage(
"You appear to be actively editing. Would you like Roo Code to proceed with file changes anyway?",
"Proceed",
"Cancel",
)

if (shouldProceed !== "Proceed") {
await cline.say("text", "File operation cancelled to avoid disrupting your work.")
pushToolResult(
formatResponse.toolError(
"File operation cancelled to avoid disrupting active editing.",
),
)
return
Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Missing await cline.diffViewProvider.reset() before returning. The editType is set at line 247 (or cached from line 74), but when the user cancels here, the state is not cleared. This could cause the cached editType to be incorrectly reused in subsequent operations (see lines 69-71). Other tools (insertContentTool line 174, searchAndReplaceTool line 245) correctly call reset() in their early return paths.

}
}
}

// Direct file write without diff view
// Check for code omissions before proceeding
if (detectCodeOmission(cline.diffViewProvider.originalContent || "", newContent, predictedLineCount)) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,24 +7,24 @@ describe("PREVENT_FOCUS_DISRUPTION experiment", () => {

it("should have PREVENT_FOCUS_DISRUPTION in experimentConfigsMap", () => {
expect(experimentConfigsMap.PREVENT_FOCUS_DISRUPTION).toBeDefined()
expect(experimentConfigsMap.PREVENT_FOCUS_DISRUPTION.enabled).toBe(false)
expect(experimentConfigsMap.PREVENT_FOCUS_DISRUPTION.enabled).toBe(true) // Enabled by default
})

it("should have PREVENT_FOCUS_DISRUPTION in experimentDefault", () => {
expect(experimentDefault.preventFocusDisruption).toBe(false)
expect(experimentDefault.preventFocusDisruption).toBe(true) // Enabled by default
})

it("should correctly check if PREVENT_FOCUS_DISRUPTION is enabled", () => {
// Test when experiment is disabled (default)
// Test when experiment is explicitly disabled
const disabledConfig = { preventFocusDisruption: false }
expect(experiments.isEnabled(disabledConfig, EXPERIMENT_IDS.PREVENT_FOCUS_DISRUPTION)).toBe(false)

// Test when experiment is enabled
// Test when experiment is explicitly enabled
const enabledConfig = { preventFocusDisruption: true }
expect(experiments.isEnabled(enabledConfig, EXPERIMENT_IDS.PREVENT_FOCUS_DISRUPTION)).toBe(true)

// Test when experiment is not in config (should use default)
// Test when experiment is not in config (should use default - true)
const emptyConfig = {}
expect(experiments.isEnabled(emptyConfig, EXPERIMENT_IDS.PREVENT_FOCUS_DISRUPTION)).toBe(false)
expect(experiments.isEnabled(emptyConfig, EXPERIMENT_IDS.PREVENT_FOCUS_DISRUPTION)).toBe(true)
})
})
2 changes: 1 addition & 1 deletion src/shared/experiments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,7 @@ interface ExperimentConfig {
export const experimentConfigsMap: Record<ExperimentKey, ExperimentConfig> = {
MULTI_FILE_APPLY_DIFF: { enabled: false },
POWER_STEERING: { enabled: false },
PREVENT_FOCUS_DISRUPTION: { enabled: false },
PREVENT_FOCUS_DISRUPTION: { enabled: true }, // Enabled by default to prevent file corruption
IMAGE_GENERATION: { enabled: false },
RUN_SLASH_COMMAND: { enabled: false },
}
Expand Down
Loading