Skip to content
Open
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
4 changes: 4 additions & 0 deletions packages/types/src/experiment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ export const experimentIds = [
"preventFocusDisruption",
"imageGeneration",
"runSlashCommand",
"preventTerminalDisruption",
"assistantMessageParser",
] as const

export const experimentIdsSchema = z.enum(experimentIds)
Expand All @@ -28,6 +30,8 @@ export const experimentsSchema = z.object({
preventFocusDisruption: z.boolean().optional(),
imageGeneration: z.boolean().optional(),
runSlashCommand: z.boolean().optional(),
preventTerminalDisruption: z.boolean().optional(),
assistantMessageParser: z.boolean().optional(),
})

export type Experiments = z.infer<typeof experimentsSchema>
Expand Down
125 changes: 124 additions & 1 deletion src/core/tools/__tests__/executeCommand.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@ describe("executeCommand", () => {
getState: vitest.fn().mockResolvedValue({
terminalOutputLineLimit: 500,
terminalShellIntegrationDisabled: false,
experiments: {},
}),
}

Expand All @@ -50,7 +51,7 @@ describe("executeCommand", () => {
cwd: "/test/project",
taskId: "test-task-123",
providerRef: {
deref: vitest.fn().mockResolvedValue(mockProvider),
deref: vitest.fn().mockReturnValue(mockProvider),
},
say: vitest.fn().mockResolvedValue(undefined),
terminalProcess: undefined,
Expand Down Expand Up @@ -451,4 +452,126 @@ describe("executeCommand", () => {
expect(mockTerminalInstance.getCurrentWorkingDirectory).toHaveBeenCalled()
})
})

describe("PREVENT_TERMINAL_DISRUPTION experiment", () => {
Copy link
Member

Choose a reason for hiding this comment

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

Add a test that simulates an interactive command when preventTerminalDisruption is enabled to ensure the output -> ask() flow still proceeds and does not strand waiting for user input while the terminal is hidden. For example, emulate a prompt line in onLine and verify the tool requests feedback and continues.

it("should not call terminal.show() when PREVENT_TERMINAL_DISRUPTION is enabled", async () => {
// Setup: Enable PREVENT_TERMINAL_DISRUPTION experiment
mockProvider.getState.mockResolvedValue({
terminalOutputLineLimit: 500,
terminalShellIntegrationDisabled: false,
experiments: { preventTerminalDisruption: true },
})

// Create a mock Terminal instance
const vscodeTerminal = new Terminal(1, undefined, "/test/project")
const mockVSCodeTerminal = vscodeTerminal as any
mockVSCodeTerminal.terminal = {
show: vitest.fn(),
}
mockVSCodeTerminal.getCurrentWorkingDirectory = vitest.fn().mockReturnValue("/test/project")
mockVSCodeTerminal.runCommand = vitest
.fn()
.mockImplementation((command: string, callbacks: RooTerminalCallbacks) => {
setTimeout(() => {
callbacks.onCompleted("Command output", mockProcess)
callbacks.onShellExecutionComplete({ exitCode: 0 }, mockProcess)
}, 0)
return mockProcess
})
;(TerminalRegistry.getOrCreateTerminal as any).mockResolvedValue(mockVSCodeTerminal)

const options: ExecuteCommandOptions = {
executionId: "test-123",
command: "echo test",
terminalShellIntegrationDisabled: false,
terminalOutputLineLimit: 500,
}

// Execute
await executeCommand(mockTask, options)

// Verify terminal.show() was NOT called
expect(mockVSCodeTerminal.terminal.show).not.toHaveBeenCalled()
})

it("should call terminal.show() when PREVENT_TERMINAL_DISRUPTION is disabled", async () => {
// Setup: Disable PREVENT_TERMINAL_DISRUPTION experiment
mockProvider.getState.mockResolvedValue({
terminalOutputLineLimit: 500,
terminalShellIntegrationDisabled: false,
experiments: { preventTerminalDisruption: false },
})

// Create a mock Terminal instance
const vscodeTerminal = new Terminal(1, undefined, "/test/project")
const mockVSCodeTerminal = vscodeTerminal as any
mockVSCodeTerminal.terminal = {
show: vitest.fn(),
}
mockVSCodeTerminal.getCurrentWorkingDirectory = vitest.fn().mockReturnValue("/test/project")
mockVSCodeTerminal.runCommand = vitest
.fn()
.mockImplementation((command: string, callbacks: RooTerminalCallbacks) => {
setTimeout(() => {
callbacks.onCompleted("Command output", mockProcess)
callbacks.onShellExecutionComplete({ exitCode: 0 }, mockProcess)
}, 0)
return mockProcess
})
;(TerminalRegistry.getOrCreateTerminal as any).mockResolvedValue(mockVSCodeTerminal)

const options: ExecuteCommandOptions = {
executionId: "test-123",
command: "echo test",
terminalShellIntegrationDisabled: false,
terminalOutputLineLimit: 500,
}

// Execute
await executeCommand(mockTask, options)

// Verify terminal.show() was called
expect(mockVSCodeTerminal.terminal.show).toHaveBeenCalledWith(true)
})

it("should call terminal.show() when PREVENT_TERMINAL_DISRUPTION is not present in experiments", async () => {
// Setup: No PREVENT_TERMINAL_DISRUPTION in experiments
mockProvider.getState.mockResolvedValue({
terminalOutputLineLimit: 500,
terminalShellIntegrationDisabled: false,
experiments: {},
})

// Create a mock Terminal instance
const vscodeTerminal = new Terminal(1, undefined, "/test/project")
const mockVSCodeTerminal = vscodeTerminal as any
mockVSCodeTerminal.terminal = {
show: vitest.fn(),
}
mockVSCodeTerminal.getCurrentWorkingDirectory = vitest.fn().mockReturnValue("/test/project")
mockVSCodeTerminal.runCommand = vitest
.fn()
.mockImplementation((command: string, callbacks: RooTerminalCallbacks) => {
setTimeout(() => {
callbacks.onCompleted("Command output", mockProcess)
callbacks.onShellExecutionComplete({ exitCode: 0 }, mockProcess)
}, 0)
return mockProcess
})
;(TerminalRegistry.getOrCreateTerminal as any).mockResolvedValue(mockVSCodeTerminal)

const options: ExecuteCommandOptions = {
executionId: "test-123",
command: "echo test",
terminalShellIntegrationDisabled: false,
terminalOutputLineLimit: 500,
}

// Execute
await executeCommand(mockTask, options)

// Verify terminal.show() was called
expect(mockVSCodeTerminal.terminal.show).toHaveBeenCalledWith(true)
})
})
})
Original file line number Diff line number Diff line change
@@ -0,0 +1,42 @@
// Integration test for PREVENT_TERMINAL_DISRUPTION functionality
// npx vitest run src/core/tools/__tests__/executeCommandTool.preventTerminalDisruption.integration.spec.ts

import { EXPERIMENT_IDS, experiments } from "../../../shared/experiments"

describe("PREVENT_TERMINAL_DISRUPTION integration", () => {
it("should have PREVENT_TERMINAL_DISRUPTION experiment defined", () => {
expect(EXPERIMENT_IDS.PREVENT_TERMINAL_DISRUPTION).toBe("preventTerminalDisruption")
})

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

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

// Test when experiment is not in config (should use default)
const emptyConfig = {}
expect(experiments.isEnabled(emptyConfig, EXPERIMENT_IDS.PREVENT_TERMINAL_DISRUPTION)).toBe(false)
})

it("should verify the executeCommandTool imports experiments correctly", async () => {
// This test verifies that the executeCommandTool module can import and use experiments
const executeCommandModule = await import("../executeCommandTool")
expect(executeCommandModule).toBeDefined()
expect(executeCommandModule.executeCommand).toBeDefined()
expect(executeCommandModule.executeCommandTool).toBeDefined()
})

it("should verify Terminal class structure for show method", async () => {
// This test verifies the Terminal class has the expected structure
const terminalModule = await import("../../../integrations/terminal/Terminal")
expect(terminalModule.Terminal).toBeDefined()

// The Terminal class should have a constructor that accepts a terminal
const Terminal = terminalModule.Terminal
expect(typeof Terminal).toBe("function")
})
})
16 changes: 15 additions & 1 deletion src/core/tools/executeCommandTool.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ import { TerminalRegistry } from "../../integrations/terminal/TerminalRegistry"
import { Terminal } from "../../integrations/terminal/Terminal"
import { Package } from "../../shared/package"
import { t } from "../../i18n"
import { EXPERIMENT_IDS, experiments } from "../../shared/experiments"

class ShellIntegrationError extends Error {}

Expand Down Expand Up @@ -241,7 +242,20 @@ export async function executeCommand(
const terminal = await TerminalRegistry.getOrCreateTerminal(workingDir, task.taskId, terminalProvider)

if (terminal instanceof Terminal) {
terminal.terminal.show(true)
// Check if PREVENT_TERMINAL_DISRUPTION is enabled
// This experimental feature allows commands to run in the background without
// automatically switching focus to the terminal output, improving workflow continuity
const provider = task.providerRef.deref()
Copy link
Member

Choose a reason for hiding this comment

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

Minor: you already read provider state in executeCommandTool earlier. You can pass a precomputed flag (e.g., preventTerminalDisruptionEnabled) into executeCommand(...) and avoid this additional getState() call. This eliminates an extra await and state drift between askApproval and execution. Also consider capturing telemetry when focus is suppressed.

const state = provider ? await provider.getState() : null
const preventTerminalDisruption = experiments.isEnabled(
state?.experiments ?? {},
EXPERIMENT_IDS.PREVENT_TERMINAL_DISRUPTION,
)

// Only show terminal if PREVENT_TERMINAL_DISRUPTION is not enabled
if (!preventTerminalDisruption) {
Copy link
Member

Choose a reason for hiding this comment

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

Consider adding an escape hatch to the options to explicitly force terminal focus when needed, e.g. forceShowTerminal?: boolean. Then honor it in the show logic. This prevents hidden terminals from breaking interactive flows (auth prompts, TUIs).

terminal.terminal.show(true)
}

// Update the working directory in case the terminal we asked for has
// a different working directory so that the model will know where the
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,30 @@
import { EXPERIMENT_IDS, experimentConfigsMap, experimentDefault, experiments } from "../experiments"

describe("PREVENT_TERMINAL_DISRUPTION experiment", () => {
it("should include PREVENT_TERMINAL_DISRUPTION in EXPERIMENT_IDS", () => {
expect(EXPERIMENT_IDS.PREVENT_TERMINAL_DISRUPTION).toBe("preventTerminalDisruption")
})

it("should have PREVENT_TERMINAL_DISRUPTION in experimentConfigsMap", () => {
expect(experimentConfigsMap.PREVENT_TERMINAL_DISRUPTION).toBeDefined()
expect(experimentConfigsMap.PREVENT_TERMINAL_DISRUPTION.enabled).toBe(false)
})

it("should have PREVENT_TERMINAL_DISRUPTION in experimentDefault", () => {
expect(experimentDefault.preventTerminalDisruption).toBe(false)
})

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

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

// Test when experiment is not in config (should use default)
const emptyConfig = {}
expect(experiments.isEnabled(emptyConfig, EXPERIMENT_IDS.PREVENT_TERMINAL_DISRUPTION)).toBe(false)
})
})
6 changes: 6 additions & 0 deletions src/shared/__tests__/experiments.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,8 @@ describe("experiments", () => {
preventFocusDisruption: false,
imageGeneration: false,
runSlashCommand: false,
preventTerminalDisruption: false,
assistantMessageParser: false,
}
expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(false)
})
Expand All @@ -42,6 +44,8 @@ describe("experiments", () => {
preventFocusDisruption: false,
imageGeneration: false,
runSlashCommand: false,
preventTerminalDisruption: false,
assistantMessageParser: false,
}
expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(true)
})
Expand All @@ -53,6 +57,8 @@ describe("experiments", () => {
preventFocusDisruption: false,
imageGeneration: false,
runSlashCommand: false,
preventTerminalDisruption: false,
assistantMessageParser: false,
}
expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(false)
})
Expand Down
4 changes: 4 additions & 0 deletions src/shared/experiments.ts
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,8 @@ export const EXPERIMENT_IDS = {
PREVENT_FOCUS_DISRUPTION: "preventFocusDisruption",
IMAGE_GENERATION: "imageGeneration",
RUN_SLASH_COMMAND: "runSlashCommand",
PREVENT_TERMINAL_DISRUPTION: "preventTerminalDisruption",
ASSISTANT_MESSAGE_PARSER: "assistantMessageParser",
} as const satisfies Record<string, ExperimentId>

type _AssertExperimentIds = AssertEqual<Equals<ExperimentId, Values<typeof EXPERIMENT_IDS>>>
Expand All @@ -22,6 +24,8 @@ export const experimentConfigsMap: Record<ExperimentKey, ExperimentConfig> = {
PREVENT_FOCUS_DISRUPTION: { enabled: false },
IMAGE_GENERATION: { enabled: false },
RUN_SLASH_COMMAND: { enabled: false },
PREVENT_TERMINAL_DISRUPTION: { enabled: false },
ASSISTANT_MESSAGE_PARSER: { enabled: false },
}

export const experimentDefault = Object.fromEntries(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -235,6 +235,8 @@ describe("mergeExtensionState", () => {
newTaskRequireTodos: false,
imageGeneration: false,
runSlashCommand: false,
preventTerminalDisruption: false,
assistantMessageParser: false,
} as Record<ExperimentId, boolean>,
}

Expand All @@ -255,6 +257,8 @@ describe("mergeExtensionState", () => {
newTaskRequireTodos: false,
imageGeneration: false,
runSlashCommand: false,
preventTerminalDisruption: false,
assistantMessageParser: false,
})
})
})
4 changes: 4 additions & 0 deletions webview-ui/src/i18n/locales/ca/settings.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions webview-ui/src/i18n/locales/de/settings.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions webview-ui/src/i18n/locales/en/settings.json
Original file line number Diff line number Diff line change
Expand Up @@ -730,6 +730,10 @@
"name": "Background editing",
"description": "Prevent editor focus disruption when enabled. File edits happen in the background without opening diff views or stealing focus. You can continue working uninterrupted while Roo makes changes. Files can be opened without focus to capture diagnostics or kept closed entirely."
},
"PREVENT_TERMINAL_DISRUPTION": {
"name": "Background terminal execution",
"description": "Prevent terminal focus disruption when enabled. Commands execute in the background without automatically switching to output terminals. You can continue working in your current terminal while Roo runs commands, maintaining your terminal context and manually controlling when to view command outputs."
},
"ASSISTANT_MESSAGE_PARSER": {
"name": "Use new message parser",
"description": "Enable the experimental streaming message parser that provides significant performance improvements for long assistant responses by processing messages more efficiently."
Expand Down
4 changes: 4 additions & 0 deletions webview-ui/src/i18n/locales/es/settings.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

4 changes: 4 additions & 0 deletions webview-ui/src/i18n/locales/fr/settings.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Loading
Loading