Skip to content

Commit 1ea05cb

Browse files
committed
fix: address PR review comments
- Fix duplicate state variable declaration - Add null check for provider.deref() - Fix Italian translation typo - Add test cases for PREVENT_TERMINAL_DISRUPTION behavior - Keep existing comment explaining the feature
1 parent 9f5e489 commit 1ea05cb

File tree

3 files changed

+125
-3
lines changed

3 files changed

+125
-3
lines changed

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

Lines changed: 122 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -472,4 +472,126 @@ describe("executeCommand", () => {
472472
expect(mockTerminalInstance.getCurrentWorkingDirectory).toHaveBeenCalled()
473473
})
474474
})
475+
476+
describe("PREVENT_TERMINAL_DISRUPTION experiment", () => {
477+
it("should not call terminal.show() when PREVENT_TERMINAL_DISRUPTION is enabled", async () => {
478+
// Setup: Enable PREVENT_TERMINAL_DISRUPTION experiment
479+
mockProvider.getState.mockResolvedValue({
480+
terminalOutputLineLimit: 500,
481+
terminalShellIntegrationDisabled: false,
482+
experiments: { preventTerminalDisruption: true },
483+
})
484+
485+
// Create a mock Terminal instance
486+
const vscodeTerminal = new Terminal(1, undefined, "/test/project")
487+
const mockVSCodeTerminal = vscodeTerminal as any
488+
mockVSCodeTerminal.terminal = {
489+
show: vitest.fn(),
490+
}
491+
mockVSCodeTerminal.getCurrentWorkingDirectory = vitest.fn().mockReturnValue("/test/project")
492+
mockVSCodeTerminal.runCommand = vitest
493+
.fn()
494+
.mockImplementation((command: string, callbacks: RooTerminalCallbacks) => {
495+
setTimeout(() => {
496+
callbacks.onCompleted("Command output", mockProcess)
497+
callbacks.onShellExecutionComplete({ exitCode: 0 }, mockProcess)
498+
}, 0)
499+
return mockProcess
500+
})
501+
;(TerminalRegistry.getOrCreateTerminal as any).mockResolvedValue(mockVSCodeTerminal)
502+
503+
const options: ExecuteCommandOptions = {
504+
executionId: "test-123",
505+
command: "echo test",
506+
terminalShellIntegrationDisabled: false,
507+
terminalOutputLineLimit: 500,
508+
}
509+
510+
// Execute
511+
await executeCommand(mockTask, options)
512+
513+
// Verify terminal.show() was NOT called
514+
expect(mockVSCodeTerminal.terminal.show).not.toHaveBeenCalled()
515+
})
516+
517+
it("should call terminal.show() when PREVENT_TERMINAL_DISRUPTION is disabled", async () => {
518+
// Setup: Disable PREVENT_TERMINAL_DISRUPTION experiment
519+
mockProvider.getState.mockResolvedValue({
520+
terminalOutputLineLimit: 500,
521+
terminalShellIntegrationDisabled: false,
522+
experiments: { preventTerminalDisruption: false },
523+
})
524+
525+
// Create a mock Terminal instance
526+
const vscodeTerminal = new Terminal(1, undefined, "/test/project")
527+
const mockVSCodeTerminal = vscodeTerminal as any
528+
mockVSCodeTerminal.terminal = {
529+
show: vitest.fn(),
530+
}
531+
mockVSCodeTerminal.getCurrentWorkingDirectory = vitest.fn().mockReturnValue("/test/project")
532+
mockVSCodeTerminal.runCommand = vitest
533+
.fn()
534+
.mockImplementation((command: string, callbacks: RooTerminalCallbacks) => {
535+
setTimeout(() => {
536+
callbacks.onCompleted("Command output", mockProcess)
537+
callbacks.onShellExecutionComplete({ exitCode: 0 }, mockProcess)
538+
}, 0)
539+
return mockProcess
540+
})
541+
;(TerminalRegistry.getOrCreateTerminal as any).mockResolvedValue(mockVSCodeTerminal)
542+
543+
const options: ExecuteCommandOptions = {
544+
executionId: "test-123",
545+
command: "echo test",
546+
terminalShellIntegrationDisabled: false,
547+
terminalOutputLineLimit: 500,
548+
}
549+
550+
// Execute
551+
await executeCommand(mockTask, options)
552+
553+
// Verify terminal.show() was called
554+
expect(mockVSCodeTerminal.terminal.show).toHaveBeenCalledWith(true)
555+
})
556+
557+
it("should call terminal.show() when PREVENT_TERMINAL_DISRUPTION is not present in experiments", async () => {
558+
// Setup: No PREVENT_TERMINAL_DISRUPTION in experiments
559+
mockProvider.getState.mockResolvedValue({
560+
terminalOutputLineLimit: 500,
561+
terminalShellIntegrationDisabled: false,
562+
experiments: {},
563+
})
564+
565+
// Create a mock Terminal instance
566+
const vscodeTerminal = new Terminal(1, undefined, "/test/project")
567+
const mockVSCodeTerminal = vscodeTerminal as any
568+
mockVSCodeTerminal.terminal = {
569+
show: vitest.fn(),
570+
}
571+
mockVSCodeTerminal.getCurrentWorkingDirectory = vitest.fn().mockReturnValue("/test/project")
572+
mockVSCodeTerminal.runCommand = vitest
573+
.fn()
574+
.mockImplementation((command: string, callbacks: RooTerminalCallbacks) => {
575+
setTimeout(() => {
576+
callbacks.onCompleted("Command output", mockProcess)
577+
callbacks.onShellExecutionComplete({ exitCode: 0 }, mockProcess)
578+
}, 0)
579+
return mockProcess
580+
})
581+
;(TerminalRegistry.getOrCreateTerminal as any).mockResolvedValue(mockVSCodeTerminal)
582+
583+
const options: ExecuteCommandOptions = {
584+
executionId: "test-123",
585+
command: "echo test",
586+
terminalShellIntegrationDisabled: false,
587+
terminalOutputLineLimit: 500,
588+
}
589+
590+
// Execute
591+
await executeCommand(mockTask, options)
592+
593+
// Verify terminal.show() was called
594+
expect(mockVSCodeTerminal.terminal.show).toHaveBeenCalledWith(true)
595+
})
596+
})
475597
})

src/core/tools/executeCommandTool.ts

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -245,8 +245,8 @@ export async function executeCommand(
245245
// Check if PREVENT_TERMINAL_DISRUPTION is enabled
246246
// This experimental feature allows commands to run in the background without
247247
// automatically switching focus to the terminal output, improving workflow continuity
248-
const state = await task.providerRef.deref()?.getState()
249-
const state = await task.providerRef.deref()?.getState()
248+
const provider = task.providerRef.deref()
249+
const state = provider ? await provider.getState() : null
250250
const preventTerminalDisruption = experiments.isEnabled(
251251
state?.experiments ?? {},
252252
EXPERIMENT_IDS.PREVENT_TERMINAL_DISRUPTION,

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

Lines changed: 1 addition & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

0 commit comments

Comments
 (0)