Skip to content

Commit 013fdc7

Browse files
committed
fix: Resolve merge conflicts for PR #6587 - combine preventTerminalDisruption experiment with existing experiments
1 parent 12f94fc commit 013fdc7

26 files changed

+301
-2
lines changed

packages/types/src/experiment.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -12,6 +12,8 @@ export const experimentIds = [
1212
"preventFocusDisruption",
1313
"imageGeneration",
1414
"runSlashCommand",
15+
"preventTerminalDisruption",
16+
"assistantMessageParser",
1517
] as const
1618

1719
export const experimentIdsSchema = z.enum(experimentIds)
@@ -28,6 +30,8 @@ export const experimentsSchema = z.object({
2830
preventFocusDisruption: z.boolean().optional(),
2931
imageGeneration: z.boolean().optional(),
3032
runSlashCommand: z.boolean().optional(),
33+
preventTerminalDisruption: z.boolean().optional(),
34+
assistantMessageParser: z.boolean().optional(),
3135
})
3236

3337
export type Experiments = z.infer<typeof experimentsSchema>

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

Lines changed: 124 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,7 @@ describe("executeCommand", () => {
4242
getState: vitest.fn().mockResolvedValue({
4343
terminalOutputLineLimit: 500,
4444
terminalShellIntegrationDisabled: false,
45+
experiments: {},
4546
}),
4647
}
4748

@@ -50,7 +51,7 @@ describe("executeCommand", () => {
5051
cwd: "/test/project",
5152
taskId: "test-task-123",
5253
providerRef: {
53-
deref: vitest.fn().mockResolvedValue(mockProvider),
54+
deref: vitest.fn().mockReturnValue(mockProvider),
5455
},
5556
say: vitest.fn().mockResolvedValue(undefined),
5657
terminalProcess: undefined,
@@ -451,4 +452,126 @@ describe("executeCommand", () => {
451452
expect(mockTerminalInstance.getCurrentWorkingDirectory).toHaveBeenCalled()
452453
})
453454
})
455+
456+
describe("PREVENT_TERMINAL_DISRUPTION experiment", () => {
457+
it("should not call terminal.show() when PREVENT_TERMINAL_DISRUPTION is enabled", async () => {
458+
// Setup: Enable PREVENT_TERMINAL_DISRUPTION experiment
459+
mockProvider.getState.mockResolvedValue({
460+
terminalOutputLineLimit: 500,
461+
terminalShellIntegrationDisabled: false,
462+
experiments: { preventTerminalDisruption: true },
463+
})
464+
465+
// Create a mock Terminal instance
466+
const vscodeTerminal = new Terminal(1, undefined, "/test/project")
467+
const mockVSCodeTerminal = vscodeTerminal as any
468+
mockVSCodeTerminal.terminal = {
469+
show: vitest.fn(),
470+
}
471+
mockVSCodeTerminal.getCurrentWorkingDirectory = vitest.fn().mockReturnValue("/test/project")
472+
mockVSCodeTerminal.runCommand = vitest
473+
.fn()
474+
.mockImplementation((command: string, callbacks: RooTerminalCallbacks) => {
475+
setTimeout(() => {
476+
callbacks.onCompleted("Command output", mockProcess)
477+
callbacks.onShellExecutionComplete({ exitCode: 0 }, mockProcess)
478+
}, 0)
479+
return mockProcess
480+
})
481+
;(TerminalRegistry.getOrCreateTerminal as any).mockResolvedValue(mockVSCodeTerminal)
482+
483+
const options: ExecuteCommandOptions = {
484+
executionId: "test-123",
485+
command: "echo test",
486+
terminalShellIntegrationDisabled: false,
487+
terminalOutputLineLimit: 500,
488+
}
489+
490+
// Execute
491+
await executeCommand(mockTask, options)
492+
493+
// Verify terminal.show() was NOT called
494+
expect(mockVSCodeTerminal.terminal.show).not.toHaveBeenCalled()
495+
})
496+
497+
it("should call terminal.show() when PREVENT_TERMINAL_DISRUPTION is disabled", async () => {
498+
// Setup: Disable PREVENT_TERMINAL_DISRUPTION experiment
499+
mockProvider.getState.mockResolvedValue({
500+
terminalOutputLineLimit: 500,
501+
terminalShellIntegrationDisabled: false,
502+
experiments: { preventTerminalDisruption: false },
503+
})
504+
505+
// Create a mock Terminal instance
506+
const vscodeTerminal = new Terminal(1, undefined, "/test/project")
507+
const mockVSCodeTerminal = vscodeTerminal as any
508+
mockVSCodeTerminal.terminal = {
509+
show: vitest.fn(),
510+
}
511+
mockVSCodeTerminal.getCurrentWorkingDirectory = vitest.fn().mockReturnValue("/test/project")
512+
mockVSCodeTerminal.runCommand = vitest
513+
.fn()
514+
.mockImplementation((command: string, callbacks: RooTerminalCallbacks) => {
515+
setTimeout(() => {
516+
callbacks.onCompleted("Command output", mockProcess)
517+
callbacks.onShellExecutionComplete({ exitCode: 0 }, mockProcess)
518+
}, 0)
519+
return mockProcess
520+
})
521+
;(TerminalRegistry.getOrCreateTerminal as any).mockResolvedValue(mockVSCodeTerminal)
522+
523+
const options: ExecuteCommandOptions = {
524+
executionId: "test-123",
525+
command: "echo test",
526+
terminalShellIntegrationDisabled: false,
527+
terminalOutputLineLimit: 500,
528+
}
529+
530+
// Execute
531+
await executeCommand(mockTask, options)
532+
533+
// Verify terminal.show() was called
534+
expect(mockVSCodeTerminal.terminal.show).toHaveBeenCalledWith(true)
535+
})
536+
537+
it("should call terminal.show() when PREVENT_TERMINAL_DISRUPTION is not present in experiments", async () => {
538+
// Setup: No PREVENT_TERMINAL_DISRUPTION in experiments
539+
mockProvider.getState.mockResolvedValue({
540+
terminalOutputLineLimit: 500,
541+
terminalShellIntegrationDisabled: false,
542+
experiments: {},
543+
})
544+
545+
// Create a mock Terminal instance
546+
const vscodeTerminal = new Terminal(1, undefined, "/test/project")
547+
const mockVSCodeTerminal = vscodeTerminal as any
548+
mockVSCodeTerminal.terminal = {
549+
show: vitest.fn(),
550+
}
551+
mockVSCodeTerminal.getCurrentWorkingDirectory = vitest.fn().mockReturnValue("/test/project")
552+
mockVSCodeTerminal.runCommand = vitest
553+
.fn()
554+
.mockImplementation((command: string, callbacks: RooTerminalCallbacks) => {
555+
setTimeout(() => {
556+
callbacks.onCompleted("Command output", mockProcess)
557+
callbacks.onShellExecutionComplete({ exitCode: 0 }, mockProcess)
558+
}, 0)
559+
return mockProcess
560+
})
561+
;(TerminalRegistry.getOrCreateTerminal as any).mockResolvedValue(mockVSCodeTerminal)
562+
563+
const options: ExecuteCommandOptions = {
564+
executionId: "test-123",
565+
command: "echo test",
566+
terminalShellIntegrationDisabled: false,
567+
terminalOutputLineLimit: 500,
568+
}
569+
570+
// Execute
571+
await executeCommand(mockTask, options)
572+
573+
// Verify terminal.show() was called
574+
expect(mockVSCodeTerminal.terminal.show).toHaveBeenCalledWith(true)
575+
})
576+
})
454577
})
Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,42 @@
1+
// Integration test for PREVENT_TERMINAL_DISRUPTION functionality
2+
// npx vitest run src/core/tools/__tests__/executeCommandTool.preventTerminalDisruption.integration.spec.ts
3+
4+
import { EXPERIMENT_IDS, experiments } from "../../../shared/experiments"
5+
6+
describe("PREVENT_TERMINAL_DISRUPTION integration", () => {
7+
it("should have PREVENT_TERMINAL_DISRUPTION experiment defined", () => {
8+
expect(EXPERIMENT_IDS.PREVENT_TERMINAL_DISRUPTION).toBe("preventTerminalDisruption")
9+
})
10+
11+
it("should correctly check if PREVENT_TERMINAL_DISRUPTION is enabled", () => {
12+
// Test when experiment is disabled (default)
13+
const disabledConfig = { preventTerminalDisruption: false }
14+
expect(experiments.isEnabled(disabledConfig, EXPERIMENT_IDS.PREVENT_TERMINAL_DISRUPTION)).toBe(false)
15+
16+
// Test when experiment is enabled
17+
const enabledConfig = { preventTerminalDisruption: true }
18+
expect(experiments.isEnabled(enabledConfig, EXPERIMENT_IDS.PREVENT_TERMINAL_DISRUPTION)).toBe(true)
19+
20+
// Test when experiment is not in config (should use default)
21+
const emptyConfig = {}
22+
expect(experiments.isEnabled(emptyConfig, EXPERIMENT_IDS.PREVENT_TERMINAL_DISRUPTION)).toBe(false)
23+
})
24+
25+
it("should verify the executeCommandTool imports experiments correctly", async () => {
26+
// This test verifies that the executeCommandTool module can import and use experiments
27+
const executeCommandModule = await import("../executeCommandTool")
28+
expect(executeCommandModule).toBeDefined()
29+
expect(executeCommandModule.executeCommand).toBeDefined()
30+
expect(executeCommandModule.executeCommandTool).toBeDefined()
31+
})
32+
33+
it("should verify Terminal class structure for show method", async () => {
34+
// This test verifies the Terminal class has the expected structure
35+
const terminalModule = await import("../../../integrations/terminal/Terminal")
36+
expect(terminalModule.Terminal).toBeDefined()
37+
38+
// The Terminal class should have a constructor that accepts a terminal
39+
const Terminal = terminalModule.Terminal
40+
expect(typeof Terminal).toBe("function")
41+
})
42+
})

src/core/tools/executeCommandTool.ts

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ import { TerminalRegistry } from "../../integrations/terminal/TerminalRegistry"
1717
import { Terminal } from "../../integrations/terminal/Terminal"
1818
import { Package } from "../../shared/package"
1919
import { t } from "../../i18n"
20+
import { EXPERIMENT_IDS, experiments } from "../../shared/experiments"
2021

2122
class ShellIntegrationError extends Error {}
2223

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

243244
if (terminal instanceof Terminal) {
244-
terminal.terminal.show(true)
245+
// Check if PREVENT_TERMINAL_DISRUPTION is enabled
246+
// This experimental feature allows commands to run in the background without
247+
// automatically switching focus to the terminal output, improving workflow continuity
248+
const provider = task.providerRef.deref()
249+
const state = provider ? await provider.getState() : null
250+
const preventTerminalDisruption = experiments.isEnabled(
251+
state?.experiments ?? {},
252+
EXPERIMENT_IDS.PREVENT_TERMINAL_DISRUPTION,
253+
)
254+
255+
// Only show terminal if PREVENT_TERMINAL_DISRUPTION is not enabled
256+
if (!preventTerminalDisruption) {
257+
terminal.terminal.show(true)
258+
}
245259

246260
// Update the working directory in case the terminal we asked for has
247261
// a different working directory so that the model will know where the
Lines changed: 30 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,30 @@
1+
import { EXPERIMENT_IDS, experimentConfigsMap, experimentDefault, experiments } from "../experiments"
2+
3+
describe("PREVENT_TERMINAL_DISRUPTION experiment", () => {
4+
it("should include PREVENT_TERMINAL_DISRUPTION in EXPERIMENT_IDS", () => {
5+
expect(EXPERIMENT_IDS.PREVENT_TERMINAL_DISRUPTION).toBe("preventTerminalDisruption")
6+
})
7+
8+
it("should have PREVENT_TERMINAL_DISRUPTION in experimentConfigsMap", () => {
9+
expect(experimentConfigsMap.PREVENT_TERMINAL_DISRUPTION).toBeDefined()
10+
expect(experimentConfigsMap.PREVENT_TERMINAL_DISRUPTION.enabled).toBe(false)
11+
})
12+
13+
it("should have PREVENT_TERMINAL_DISRUPTION in experimentDefault", () => {
14+
expect(experimentDefault.preventTerminalDisruption).toBe(false)
15+
})
16+
17+
it("should correctly check if PREVENT_TERMINAL_DISRUPTION is enabled", () => {
18+
// Test when experiment is disabled (default)
19+
const disabledConfig = { preventTerminalDisruption: false }
20+
expect(experiments.isEnabled(disabledConfig, EXPERIMENT_IDS.PREVENT_TERMINAL_DISRUPTION)).toBe(false)
21+
22+
// Test when experiment is enabled
23+
const enabledConfig = { preventTerminalDisruption: true }
24+
expect(experiments.isEnabled(enabledConfig, EXPERIMENT_IDS.PREVENT_TERMINAL_DISRUPTION)).toBe(true)
25+
26+
// Test when experiment is not in config (should use default)
27+
const emptyConfig = {}
28+
expect(experiments.isEnabled(emptyConfig, EXPERIMENT_IDS.PREVENT_TERMINAL_DISRUPTION)).toBe(false)
29+
})
30+
})

src/shared/__tests__/experiments.spec.ts

Lines changed: 6 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,8 @@ describe("experiments", () => {
3131
preventFocusDisruption: false,
3232
imageGeneration: false,
3333
runSlashCommand: false,
34+
preventTerminalDisruption: false,
35+
assistantMessageParser: false,
3436
}
3537
expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(false)
3638
})
@@ -42,6 +44,8 @@ describe("experiments", () => {
4244
preventFocusDisruption: false,
4345
imageGeneration: false,
4446
runSlashCommand: false,
47+
preventTerminalDisruption: false,
48+
assistantMessageParser: false,
4549
}
4650
expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(true)
4751
})
@@ -53,6 +57,8 @@ describe("experiments", () => {
5357
preventFocusDisruption: false,
5458
imageGeneration: false,
5559
runSlashCommand: false,
60+
preventTerminalDisruption: false,
61+
assistantMessageParser: false,
5662
}
5763
expect(Experiments.isEnabled(experiments, EXPERIMENT_IDS.POWER_STEERING)).toBe(false)
5864
})

src/shared/experiments.ts

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,8 @@ export const EXPERIMENT_IDS = {
66
PREVENT_FOCUS_DISRUPTION: "preventFocusDisruption",
77
IMAGE_GENERATION: "imageGeneration",
88
RUN_SLASH_COMMAND: "runSlashCommand",
9+
PREVENT_TERMINAL_DISRUPTION: "preventTerminalDisruption",
10+
ASSISTANT_MESSAGE_PARSER: "assistantMessageParser",
911
} as const satisfies Record<string, ExperimentId>
1012

1113
type _AssertExperimentIds = AssertEqual<Equals<ExperimentId, Values<typeof EXPERIMENT_IDS>>>
@@ -22,6 +24,8 @@ export const experimentConfigsMap: Record<ExperimentKey, ExperimentConfig> = {
2224
PREVENT_FOCUS_DISRUPTION: { enabled: false },
2325
IMAGE_GENERATION: { enabled: false },
2426
RUN_SLASH_COMMAND: { enabled: false },
27+
PREVENT_TERMINAL_DISRUPTION: { enabled: false },
28+
ASSISTANT_MESSAGE_PARSER: { enabled: false },
2529
}
2630

2731
export const experimentDefault = Object.fromEntries(

webview-ui/src/context/__tests__/ExtensionStateContext.spec.tsx

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -235,6 +235,8 @@ describe("mergeExtensionState", () => {
235235
newTaskRequireTodos: false,
236236
imageGeneration: false,
237237
runSlashCommand: false,
238+
preventTerminalDisruption: false,
239+
assistantMessageParser: false,
238240
} as Record<ExperimentId, boolean>,
239241
}
240242

@@ -255,6 +257,8 @@ describe("mergeExtensionState", () => {
255257
newTaskRequireTodos: false,
256258
imageGeneration: false,
257259
runSlashCommand: false,
260+
preventTerminalDisruption: false,
261+
assistantMessageParser: false,
258262
})
259263
})
260264
})

webview-ui/src/i18n/locales/ca/settings.json

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

webview-ui/src/i18n/locales/de/settings.json

Lines changed: 4 additions & 0 deletions
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)