Skip to content

Commit 2430e8c

Browse files
committed
feat: prevent file corruption from focus switching during active editing
- Add ActivityDetector utility to track user typing/editing activity - Integrate activity detection into all file editing tools - Wait for 2-second inactivity timeout before file operations - Show warning dialog if user remains active after 10 seconds - Enable PREVENT_FOCUS_DISRUPTION experiment by default - Add comprehensive tests for activity detection Fixes #8840
1 parent f5d7ba1 commit 2430e8c

File tree

10 files changed

+574
-7
lines changed

10 files changed

+574
-7
lines changed

src/core/tools/__tests__/insertContentTool.spec.ts

Lines changed: 20 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -5,6 +5,7 @@ import type { MockedFunction } from "vitest"
55
import { fileExistsAtPath } from "../../../utils/fs"
66
import { ToolUse, ToolResponse } from "../../../shared/tools"
77
import { insertContentTool } from "../insertContentTool"
8+
import { experiments } from "../../../shared/experiments"
89

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

27+
vi.mock("../../../shared/experiments", () => ({
28+
experiments: {
29+
isEnabled: vi.fn().mockReturnValue(false),
30+
},
31+
EXPERIMENT_IDS: {
32+
PREVENT_FOCUS_DISRUPTION: "preventFocusDisruption",
33+
},
34+
}))
35+
36+
vi.mock("../../../utils/activity-detector", () => ({
37+
getActivityDetector: vi.fn().mockReturnValue({
38+
isUserActive: vi.fn().mockReturnValue(false),
39+
waitForInactivity: vi.fn().mockResolvedValue(true),
40+
}),
41+
}))
42+
2643
vi.mock("../../prompts/responses", () => ({
2744
formatResponse: {
2845
toolError: vi.fn((msg) => `Error: ${msg}`),
@@ -64,6 +81,9 @@ describe("insertContentTool", () => {
6481
beforeEach(() => {
6582
vi.clearAllMocks()
6683

84+
// Mock experiments to be disabled by default for tests
85+
vi.mocked(experiments).isEnabled.mockReturnValue(false)
86+
6787
mockedFileExistsAtPath.mockResolvedValue(true) // Assume file exists by default for insert
6888
mockedFsReadFile.mockResolvedValue("") // Default empty file content
6989

src/core/tools/__tests__/writeToFileTool.spec.ts

Lines changed: 13 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import { unescapeHtmlEntities } from "../../../utils/text-normalization"
1010
import { everyLineHasLineNumbers, stripLineNumbers } from "../../../integrations/misc/extract-text"
1111
import { ToolUse, ToolResponse } from "../../../shared/tools"
1212
import { writeToFileTool } from "../writeToFileTool"
13+
import { experiments } from "../../../shared/experiments"
1314

1415
vi.mock("path", async () => {
1516
const originalPath = await vi.importActual("path")
@@ -92,6 +93,15 @@ vi.mock("../../ignore/RooIgnoreController", () => ({
9293
},
9394
}))
9495

96+
vi.mock("../../../shared/experiments", () => ({
97+
EXPERIMENT_IDS: {
98+
PREVENT_FOCUS_DISRUPTION: "preventFocusDisruption",
99+
},
100+
experiments: {
101+
isEnabled: vi.fn().mockReturnValue(false), // Default to disabled for tests
102+
},
103+
}))
104+
95105
describe("writeToFileTool", () => {
96106
// Test data
97107
const testFilePath = "test/file.txt"
@@ -119,6 +129,9 @@ describe("writeToFileTool", () => {
119129
beforeEach(() => {
120130
vi.clearAllMocks()
121131

132+
// Mock experiments to be disabled by default for tests
133+
vi.mocked(experiments.isEnabled).mockReturnValue(false)
134+
122135
mockedPathResolve.mockReturnValue(absoluteFilePath)
123136
mockedFileExistsAtPath.mockResolvedValue(false)
124137
mockedDetectCodeOmission.mockReturnValue(false)

src/core/tools/applyDiffTool.ts

Lines changed: 27 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
import path from "path"
22
import fs from "fs/promises"
3+
import * as vscode from "vscode"
34

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

1719
export async function applyDiffToolLegacy(
1820
cline: Task,
@@ -154,6 +156,31 @@ export async function applyDiffToolLegacy(
154156
const isWriteProtected = cline.rooProtectedController?.isWriteProtected(relPath) || false
155157

156158
if (isPreventFocusDisruptionEnabled) {
159+
// Wait for user to become inactive before proceeding with file operations
160+
const activityDetector = getActivityDetector()
161+
if (activityDetector.isUserActive()) {
162+
// Wait up to 10 seconds for user to become inactive
163+
const becameInactive = await activityDetector.waitForInactivity(10000)
164+
165+
if (!becameInactive) {
166+
// User is still active after timeout, ask for permission to proceed
167+
const shouldProceed = await vscode.window.showWarningMessage(
168+
"You appear to be actively editing. Would you like Roo Code to proceed with file changes anyway?",
169+
"Proceed",
170+
"Cancel",
171+
)
172+
173+
if (shouldProceed !== "Proceed") {
174+
pushToolResult(
175+
formatResponse.toolError(
176+
"File operation cancelled to avoid disrupting active editing.",
177+
),
178+
)
179+
return
180+
}
181+
}
182+
}
183+
157184
// Direct file write without diff view
158185
const completeMessage = JSON.stringify({
159186
...sharedMessageProps,

src/core/tools/insertContentTool.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
import delay from "delay"
22
import fs from "fs/promises"
33
import path from "path"
4+
import * as vscode from "vscode"
45

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

1618
export async function insertContentTool(
1719
cline: Task,
@@ -150,6 +152,30 @@ export async function insertContentTool(
150152
await cline.diffViewProvider.open(relPath)
151153
await cline.diffViewProvider.update(updatedContent, true)
152154
cline.diffViewProvider.scrollToFirstDiff()
155+
} else {
156+
// Wait for user to become inactive before proceeding with file operations
157+
const activityDetector = getActivityDetector()
158+
if (activityDetector.isUserActive()) {
159+
// Wait up to 10 seconds for user to become inactive
160+
const becameInactive = await activityDetector.waitForInactivity(10000)
161+
162+
if (!becameInactive) {
163+
// User is still active after timeout, ask for permission to proceed
164+
const shouldProceed = await vscode.window.showWarningMessage(
165+
"You appear to be actively editing. Would you like Roo Code to proceed with file changes anyway?",
166+
"Proceed",
167+
"Cancel",
168+
)
169+
170+
if (shouldProceed !== "Proceed") {
171+
pushToolResult(
172+
formatResponse.toolError("File operation cancelled to avoid disrupting active editing."),
173+
)
174+
await cline.diffViewProvider.reset()
175+
return
176+
}
177+
}
178+
}
153179
}
154180

155181
// Ask for approval (same for both flows)

src/core/tools/searchAndReplaceTool.ts

Lines changed: 26 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22
import path from "path"
33
import fs from "fs/promises"
44
import delay from "delay"
5+
import * as vscode from "vscode"
56

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

1719
/**
1820
* Tool for performing search and replace operations on files
@@ -221,6 +223,30 @@ export async function searchAndReplaceTool(
221223
await cline.diffViewProvider.open(validRelPath)
222224
await cline.diffViewProvider.update(newContent, true)
223225
cline.diffViewProvider.scrollToFirstDiff()
226+
} else {
227+
// Wait for user to become inactive before proceeding with file operations
228+
const activityDetector = getActivityDetector()
229+
if (activityDetector.isUserActive()) {
230+
// Wait up to 10 seconds for user to become inactive
231+
const becameInactive = await activityDetector.waitForInactivity(10000)
232+
233+
if (!becameInactive) {
234+
// User is still active after timeout, ask for permission to proceed
235+
const shouldProceed = await vscode.window.showWarningMessage(
236+
"You appear to be actively editing. Would you like Roo Code to proceed with file changes anyway?",
237+
"Proceed",
238+
"Cancel",
239+
)
240+
241+
if (shouldProceed !== "Proceed") {
242+
pushToolResult(
243+
formatResponse.toolError("File operation cancelled to avoid disrupting active editing."),
244+
)
245+
await cline.diffViewProvider.reset()
246+
return
247+
}
248+
}
249+
}
224250
}
225251

226252
const didApprove = await askApproval("tool", completeMessage, undefined, isWriteProtected)

src/core/tools/writeToFileTool.ts

Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@ import { detectCodeOmission } from "../../integrations/editor/detect-omission"
1616
import { unescapeHtmlEntities } from "../../utils/text-normalization"
1717
import { DEFAULT_WRITE_DELAY_MS } from "@roo-code/types"
1818
import { EXPERIMENT_IDS, experiments } from "../../shared/experiments"
19+
import { getActivityDetector } from "../../utils/activity-detector"
1920

2021
export async function writeToFileTool(
2122
cline: Task,
@@ -172,6 +173,35 @@ export async function writeToFileTool(
172173
)
173174

174175
if (isPreventFocusDisruptionEnabled) {
176+
// Wait for user to become inactive before proceeding with file operations
177+
const activityDetector = getActivityDetector()
178+
if (activityDetector.isUserActive()) {
179+
// Notify user that we're waiting for them to stop typing
180+
await cline.say("text", "Waiting for you to finish typing before making file changes...")
181+
182+
// Wait up to 10 seconds for user to become inactive
183+
const becameInactive = await activityDetector.waitForInactivity(10000)
184+
185+
if (!becameInactive) {
186+
// User is still active after timeout, ask for permission to proceed
187+
const shouldProceed = await vscode.window.showWarningMessage(
188+
"You appear to be actively editing. Would you like Roo Code to proceed with file changes anyway?",
189+
"Proceed",
190+
"Cancel",
191+
)
192+
193+
if (shouldProceed !== "Proceed") {
194+
await cline.say("text", "File operation cancelled to avoid disrupting your work.")
195+
pushToolResult(
196+
formatResponse.toolError(
197+
"File operation cancelled to avoid disrupting active editing.",
198+
),
199+
)
200+
return
201+
}
202+
}
203+
}
204+
175205
// Direct file write without diff view
176206
// Check for code omissions before proceeding
177207
if (detectCodeOmission(cline.diffViewProvider.originalContent || "", newContent, predictedLineCount)) {

src/shared/__tests__/experiments-preventFocusDisruption.spec.ts

Lines changed: 6 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -7,24 +7,24 @@ describe("PREVENT_FOCUS_DISRUPTION experiment", () => {
77

88
it("should have PREVENT_FOCUS_DISRUPTION in experimentConfigsMap", () => {
99
expect(experimentConfigsMap.PREVENT_FOCUS_DISRUPTION).toBeDefined()
10-
expect(experimentConfigsMap.PREVENT_FOCUS_DISRUPTION.enabled).toBe(false)
10+
expect(experimentConfigsMap.PREVENT_FOCUS_DISRUPTION.enabled).toBe(true) // Enabled by default
1111
})
1212

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

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

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

26-
// Test when experiment is not in config (should use default)
26+
// Test when experiment is not in config (should use default - true)
2727
const emptyConfig = {}
28-
expect(experiments.isEnabled(emptyConfig, EXPERIMENT_IDS.PREVENT_FOCUS_DISRUPTION)).toBe(false)
28+
expect(experiments.isEnabled(emptyConfig, EXPERIMENT_IDS.PREVENT_FOCUS_DISRUPTION)).toBe(true)
2929
})
3030
})

src/shared/experiments.ts

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@ interface ExperimentConfig {
1919
export const experimentConfigsMap: Record<ExperimentKey, ExperimentConfig> = {
2020
MULTI_FILE_APPLY_DIFF: { enabled: false },
2121
POWER_STEERING: { enabled: false },
22-
PREVENT_FOCUS_DISRUPTION: { enabled: false },
22+
PREVENT_FOCUS_DISRUPTION: { enabled: true }, // Enabled by default to prevent file corruption
2323
IMAGE_GENERATION: { enabled: false },
2424
RUN_SLASH_COMMAND: { enabled: false },
2525
}

0 commit comments

Comments
 (0)