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
This commit is contained in:
Roo Code 2025-10-26 11:29:18 +00:00
parent f5d7ba1959
commit 2430e8c4c4
10 changed files with 574 additions and 7 deletions

View file

@ -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, "/")
@ -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}`),
@ -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

View file

@ -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")
@ -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"
@ -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)

View file

@ -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"
@ -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,
@ -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,

View file

@ -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"
@ -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,
@ -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)

View file

@ -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"
@ -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
@ -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)

View file

@ -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,
@ -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
}
}
}
// Direct file write without diff view
// Check for code omissions before proceeding
if (detectCodeOmission(cline.diffViewProvider.originalContent || "", newContent, predictedLineCount)) {

View file

@ -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)
})
})

View file

@ -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 },
}

View file

@ -0,0 +1,302 @@
import { describe, it, expect, vi, beforeEach, afterEach } from "vitest"
import * as vscode from "vscode"
import { ActivityDetector, getActivityDetector } from "../activity-detector"
// Mock vscode module
vi.mock("vscode", () => ({
workspace: {
onDidChangeTextDocument: vi.fn((callback) => ({
dispose: vi.fn(),
})),
},
window: {
onDidChangeTextEditorSelection: vi.fn((callback) => ({
dispose: vi.fn(),
})),
onDidChangeActiveTextEditor: vi.fn((callback) => ({
dispose: vi.fn(),
})),
showWarningMessage: vi.fn(),
},
TextDocumentChangeReason: {
Undo: 1,
Redo: 2,
},
TextEditorSelectionChangeKind: {
Keyboard: 1,
Mouse: 2,
Command: 3,
},
}))
describe("ActivityDetector", () => {
let detector: ActivityDetector
let mockDisposable: { dispose: ReturnType<typeof vi.fn> }
beforeEach(() => {
vi.clearAllMocks()
vi.useFakeTimers()
// Setup mock disposable
mockDisposable = { dispose: vi.fn() }
// Setup event handler mocks to return disposables
vi.mocked(vscode.workspace.onDidChangeTextDocument).mockReturnValue(mockDisposable)
vi.mocked(vscode.window.onDidChangeTextEditorSelection).mockReturnValue(mockDisposable)
vi.mocked(vscode.window.onDidChangeActiveTextEditor).mockReturnValue(mockDisposable)
// Reset singleton instance
if (ActivityDetector["instance"]) {
ActivityDetector["instance"]?.dispose()
}
detector = ActivityDetector.getInstance()
})
afterEach(() => {
detector.dispose()
vi.useRealTimers()
})
describe("getInstance", () => {
it("should return the same instance when called multiple times", () => {
const instance1 = ActivityDetector.getInstance()
const instance2 = ActivityDetector.getInstance()
expect(instance1).toBe(instance2)
})
it("should create a new instance after disposal", () => {
const instance1 = ActivityDetector.getInstance()
instance1.dispose()
const instance2 = ActivityDetector.getInstance()
expect(instance1).not.toBe(instance2)
})
})
describe("Activity detection", () => {
it("should detect activity on text document changes", () => {
// Get the callback registered for text document changes
const onChangeCallback = vi.mocked(vscode.workspace.onDidChangeTextDocument).mock.calls[0][0]
// Initially should not be active
expect(detector.isUserActive()).toBe(false)
// Simulate user typing (no reason means user input)
onChangeCallback({
reason: undefined,
document: {} as any,
contentChanges: [],
})
expect(detector.isUserActive()).toBe(true)
})
it("should detect activity on undo/redo operations", () => {
const onChangeCallback = vi.mocked(vscode.workspace.onDidChangeTextDocument).mock.calls[0][0]
// Simulate undo operation
onChangeCallback({
reason: vscode.TextDocumentChangeReason.Undo,
document: {} as any,
contentChanges: [],
})
expect(detector.isUserActive()).toBe(true)
})
it("should detect activity on cursor movement", () => {
const onSelectionCallback = vi.mocked(vscode.window.onDidChangeTextEditorSelection).mock.calls[0][0]
// Simulate keyboard cursor movement
onSelectionCallback({
kind: vscode.TextEditorSelectionChangeKind.Keyboard,
textEditor: {} as any,
selections: [],
})
expect(detector.isUserActive()).toBe(true)
})
it("should detect activity on mouse selection", () => {
const onSelectionCallback = vi.mocked(vscode.window.onDidChangeTextEditorSelection).mock.calls[0][0]
// Simulate mouse selection
onSelectionCallback({
kind: vscode.TextEditorSelectionChangeKind.Mouse,
textEditor: {} as any,
selections: [],
})
expect(detector.isUserActive()).toBe(true)
})
it("should not detect activity on command-based selection changes", () => {
const onSelectionCallback = vi.mocked(vscode.window.onDidChangeTextEditorSelection).mock.calls[0][0]
// Simulate command-based selection (programmatic)
onSelectionCallback({
kind: vscode.TextEditorSelectionChangeKind.Command,
textEditor: {} as any,
selections: [],
})
expect(detector.isUserActive()).toBe(false)
})
it("should detect activity on active editor changes", () => {
const onEditorChangeCallback = vi.mocked(vscode.window.onDidChangeActiveTextEditor).mock.calls[0][0]
// Simulate editor change
onEditorChangeCallback({} as any)
expect(detector.isUserActive()).toBe(true)
})
})
describe("Inactivity detection", () => {
it("should report inactive after timeout period", () => {
const onChangeCallback = vi.mocked(vscode.workspace.onDidChangeTextDocument).mock.calls[0][0]
// Trigger activity
onChangeCallback({
reason: undefined,
document: {} as any,
contentChanges: [],
})
expect(detector.isUserActive()).toBe(true)
// Advance time by 1.5 seconds (less than timeout)
vi.advanceTimersByTime(1500)
expect(detector.isUserActive()).toBe(true)
// Advance time by another 1 second (total 2.5 seconds, more than timeout)
vi.advanceTimersByTime(1000)
expect(detector.isUserActive()).toBe(false)
})
it("should return correct time since last activity", () => {
const onChangeCallback = vi.mocked(vscode.workspace.onDidChangeTextDocument).mock.calls[0][0]
// Trigger activity
onChangeCallback({
reason: undefined,
document: {} as any,
contentChanges: [],
})
// Initially should be ~0
expect(detector.getTimeSinceLastActivity()).toBeLessThan(10)
// Advance time by 1 second
vi.advanceTimersByTime(1000)
expect(detector.getTimeSinceLastActivity()).toBeGreaterThanOrEqual(1000)
expect(detector.getTimeSinceLastActivity()).toBeLessThan(1100)
})
})
describe("waitForInactivity", () => {
it("should resolve immediately if user is already inactive", async () => {
// User is inactive by default
const promise = detector.waitForInactivity(5000)
// Advance timers slightly to process promise
vi.advanceTimersByTime(100)
const result = await promise
expect(result).toBe(true)
})
it("should wait for user to become inactive", async () => {
const onChangeCallback = vi.mocked(vscode.workspace.onDidChangeTextDocument).mock.calls[0][0]
// Make user active
onChangeCallback({
reason: undefined,
document: {} as any,
contentChanges: [],
})
const promise = detector.waitForInactivity(5000)
// Advance time by 1.5 seconds (user still active)
vi.advanceTimersByTime(1500)
// Advance time by 1 second more (total 2.5 seconds, user now inactive)
vi.advanceTimersByTime(1000)
// Process promise resolution
vi.advanceTimersByTime(100)
const result = await promise
expect(result).toBe(true)
})
it("should timeout if user remains active", async () => {
const onChangeCallback = vi.mocked(vscode.workspace.onDidChangeTextDocument).mock.calls[0][0]
// Make user active
onChangeCallback({
reason: undefined,
document: {} as any,
contentChanges: [],
})
const promise = detector.waitForInactivity(3000)
// Keep user active by triggering activity every second
const interval = setInterval(() => {
onChangeCallback({
reason: undefined,
document: {} as any,
contentChanges: [],
})
}, 1000)
// Advance time beyond max wait time
vi.advanceTimersByTime(3500)
clearInterval(interval)
const result = await promise
expect(result).toBe(false)
})
})
describe("dispose", () => {
it("should dispose all event listeners", () => {
detector.dispose()
// Check that all disposables were called
expect(mockDisposable.dispose).toHaveBeenCalledTimes(3) // One for each event listener
})
it("should clear the singleton instance", () => {
detector.dispose()
// After disposal, getInstance should create a new instance
const newDetector = ActivityDetector.getInstance()
expect(newDetector).not.toBe(detector)
// Clean up
newDetector.dispose()
})
})
})
describe("getActivityDetector", () => {
afterEach(() => {
ActivityDetector.getInstance().dispose()
})
it("should return an ActivityDetector instance", () => {
const detector = getActivityDetector()
expect(detector).toBeInstanceOf(ActivityDetector)
})
it("should return the same instance as getInstance", () => {
const detector1 = getActivityDetector()
const detector2 = ActivityDetector.getInstance()
expect(detector1).toBe(detector2)
})
})

View file

@ -0,0 +1,123 @@
import * as vscode from "vscode"
/**
* ActivityDetector tracks user activity in the editor to prevent disruption
* when the user is actively typing or editing.
*/
export class ActivityDetector {
private static instance: ActivityDetector | undefined
private lastActivityTime: number = 0
private disposables: vscode.Disposable[] = []
private readonly ACTIVITY_TIMEOUT_MS = 2000 // Consider user inactive after 2 seconds
private constructor() {
this.setupListeners()
}
/**
* Get the singleton instance of ActivityDetector
*/
static getInstance(): ActivityDetector {
if (!ActivityDetector.instance) {
ActivityDetector.instance = new ActivityDetector()
}
return ActivityDetector.instance
}
/**
* Setup event listeners to track user activity
*/
private setupListeners() {
// Track text document changes (typing)
this.disposables.push(
vscode.workspace.onDidChangeTextDocument((event) => {
// Only track changes from user input, not programmatic changes
if (
event.reason === undefined ||
event.reason === vscode.TextDocumentChangeReason.Undo ||
event.reason === vscode.TextDocumentChangeReason.Redo
) {
this.updateActivityTime()
}
}),
)
// Track selection changes (cursor movement)
this.disposables.push(
vscode.window.onDidChangeTextEditorSelection((event) => {
// Only track if the change was triggered by keyboard or mouse
if (
event.kind === vscode.TextEditorSelectionChangeKind.Keyboard ||
event.kind === vscode.TextEditorSelectionChangeKind.Mouse
) {
this.updateActivityTime()
}
}),
)
// Track active editor changes
this.disposables.push(
vscode.window.onDidChangeActiveTextEditor(() => {
this.updateActivityTime()
}),
)
}
/**
* Update the last activity timestamp
*/
private updateActivityTime() {
this.lastActivityTime = Date.now()
}
/**
* Check if the user is currently active (has been active within the timeout period)
*/
isUserActive(): boolean {
return Date.now() - this.lastActivityTime < this.ACTIVITY_TIMEOUT_MS
}
/**
* Get the time in milliseconds since the last user activity
*/
getTimeSinceLastActivity(): number {
return Date.now() - this.lastActivityTime
}
/**
* Wait for user to become inactive before proceeding
* @param maxWaitMs Maximum time to wait in milliseconds (default: 5000ms)
* @returns Promise that resolves when user becomes inactive or timeout is reached
*/
async waitForInactivity(maxWaitMs: number = 5000): Promise<boolean> {
const startTime = Date.now()
while (this.isUserActive()) {
// Check if we've exceeded max wait time
if (Date.now() - startTime > maxWaitMs) {
return false // Timeout reached, user still active
}
// Wait a bit before checking again
await new Promise((resolve) => setTimeout(resolve, 100))
}
return true // User is now inactive
}
/**
* Dispose of all event listeners
*/
dispose() {
this.disposables.forEach((d) => d.dispose())
this.disposables = []
ActivityDetector.instance = undefined
}
}
/**
* Get the global activity detector instance
*/
export function getActivityDetector(): ActivityDetector {
return ActivityDetector.getInstance()
}