Commit f6985826 authored by Olena Horal-Koretska's avatar Olena Horal-Koretska 2️⃣ Committed by Elwyn Benson
Browse files

fix: Run terminal command in the background

parent cfc7013a
Loading
Loading
Loading
Loading
+72 −5
Original line number Diff line number Diff line
@@ -115,7 +115,7 @@ describe('Terminal Manager', () => {
      return result;
    });

    it('disposes all terminals and request lisener', async () => {
    it('disposes all terminals and request listener', async () => {
      terminalManager.dispose();

      expect(listenerDispose).toHaveBeenCalled();
@@ -127,6 +127,7 @@ describe('Terminal Manager', () => {
  describe('$/gitlab/runCommand', () => {
    const workflowId = '1234';
    const command = 'npm';
    const silent = false;
    const args = ['run', 'test:unit'];
    let eventFn: RunCommandEventHandler;
    let shellIntegrationFn: (
@@ -178,7 +179,7 @@ describe('Terminal Manager', () => {
        }
      });

      const result = eventFn({ workflowId, command, args });
      const result = eventFn({ workflowId, command, args, silent });

      shellIntegrationFn({ terminal: mockTerminal, shellIntegration: mockShellIntegration });

@@ -196,6 +197,7 @@ describe('Terminal Manager', () => {
      expect(window.createTerminal).toHaveBeenCalledWith({
        name: `GitLab Duo Agent Platform ${workflowId}`,
        isTransient: true,
        hideFromUser: silent,
      });

      expect(window.onDidChangeTerminalShellIntegration).toHaveBeenCalled();
@@ -212,14 +214,14 @@ describe('Terminal Manager', () => {
      expect(exitCode).toBe(0);
    });

    it('reuses a terminal when the same workflow ID is used', async () => {
    it('reuses a terminal when the same workflow ID and silent flag are used', async () => {
      mockShellIntegrationGetter.mockReturnValue(mockShellIntegration);

      jest.mocked(mockExecution.read).mockImplementation(async function* read() {
        yield '';
      });

      const firstResult = eventFn({ workflowId, command, args });
      const firstResult = eventFn({ workflowId, command, args, silent });

      await jest.advanceTimersToNextTimerAsync();

@@ -232,7 +234,7 @@ describe('Terminal Manager', () => {

      await firstResult;

      const secondResult = eventFn({ workflowId, command, args });
      const secondResult = eventFn({ workflowId, command, args, silent });

      await jest.advanceTimersToNextTimerAsync();

@@ -248,6 +250,71 @@ describe('Terminal Manager', () => {
      expect(window.createTerminal).toHaveBeenCalledTimes(1);
    });

    it('creates separate terminals for same workflow ID but different silent flags', async () => {
      const mockTerminalSilent = createFakePartial<Terminal>({
        get shellIntegration() {
          return mockShellIntegrationGetter();
        },
        show: jest.fn(),
        dispose: jest.fn(),
      });

      jest
        .mocked(window.createTerminal)
        .mockReturnValueOnce(mockTerminal) // First call returns regular terminal
        .mockReturnValueOnce(mockTerminalSilent); // Second call returns silent terminal

      mockShellIntegrationGetter.mockReturnValue(mockShellIntegration);

      jest.mocked(mockExecution.read).mockImplementation(async function* read() {
        yield '';
      });

      // First request with silent = false
      const firstResult = eventFn({ workflowId, command, args, silent: false });

      await jest.advanceTimersToNextTimerAsync();

      shellExecutionFn({
        execution: mockExecution,
        exitCode: 0,
        terminal: mockTerminal,
        shellIntegration: mockShellIntegration,
      });

      await firstResult;

      // Second request with same workflowId but silent = true
      const secondResult = eventFn({ workflowId, command, args, silent: true });

      await jest.advanceTimersToNextTimerAsync();

      shellExecutionFn({
        execution: mockExecution,
        exitCode: 0,
        terminal: mockTerminalSilent,
        shellIntegration: mockShellIntegration,
      });

      await secondResult;

      // Should create two terminals because silent flag is different
      expect(window.createTerminal).toHaveBeenCalledTimes(2);
      expect(window.createTerminal).toHaveBeenNthCalledWith(1, {
        name: `GitLab Duo Agent Platform ${workflowId}`,
        isTransient: true,
        hideFromUser: false,
      });
      expect(window.createTerminal).toHaveBeenNthCalledWith(2, {
        name: `GitLab Duo Agent Platform ${workflowId}`,
        isTransient: true,
        hideFromUser: true,
      });

      expect(mockTerminal.show).toHaveBeenCalled();
      expect(mockTerminalSilent.show).not.toHaveBeenCalled();
    });

    it('re-creates terminal if the terminal is closed', async () => {
      mockShellIntegrationGetter.mockReturnValue(mockShellIntegration);

+17 −8
Original line number Diff line number Diff line
@@ -3,6 +3,10 @@ import { BaseLanguageClient } from 'vscode-languageclient';
import { createInterfaceId } from '@gitlab/needle';
import { log } from '../log';

const lookupKey = (workflowId: string, silent: boolean) => {
  return `${workflowId}-${silent}`;
};

export interface TerminalManager extends Disposable {
  setupRequests(client: BaseLanguageClient): void;
}
@@ -20,10 +24,10 @@ export class TerminalManagerImpl implements TerminalManager {

  setupRequests(client: BaseLanguageClient) {
    this.#disposables.push(
      client.onRequest('$/gitlab/runCommand', async ({ workflowId, command, args }) => {
      client.onRequest('$/gitlab/runCommand', async ({ workflowId, command, args, silent }) => {
        log.debug(`Running command: ${command} ${args.join(' ')}`);

        return this.#executeCommand(workflowId, command, args);
        return this.#executeCommand(workflowId, command, args, silent);
      }),
    );

@@ -39,14 +43,16 @@ export class TerminalManagerImpl implements TerminalManager {
    );
  }

  async #executeCommand(workflowId: string, command: string, args: string[]) {
    const terminal = await this.#getOrCreateTerminal(workflowId);
  async #executeCommand(workflowId: string, command: string, args: string[], silent: boolean) {
    const terminal = await this.#getOrCreateTerminal(workflowId, silent);

    if (!terminal.shellIntegration) {
      throw new Error('User does not have shell integration configured');
    }

    if (!silent) {
      terminal.show(true);
    }

    const exec = terminal.shellIntegration.executeCommand(command, args);
    const executionEnd = new Promise(resolve => {
@@ -69,17 +75,20 @@ export class TerminalManagerImpl implements TerminalManager {
    return { exitCode, output: output.join('') };
  }

  async #getOrCreateTerminal(workflowId: string) {
    return this.#terminals.get(workflowId) ?? this.#createTerminal(workflowId);
  async #getOrCreateTerminal(workflowId: string, silent: boolean) {
    return (
      this.#terminals.get(lookupKey(workflowId, silent)) ?? this.#createTerminal(workflowId, silent)
    );
  }

  async #createTerminal(workflowId: string) {
  async #createTerminal(workflowId: string, silent: boolean) {
    const terminal = window.createTerminal({
      name: `GitLab Duo Agent Platform ${workflowId}`,
      isTransient: true,
      hideFromUser: silent,
    });

    this.#terminals.set(workflowId, terminal);
    this.#terminals.set(lookupKey(workflowId, silent), terminal);

    await this.#listenForShellIntegration(terminal);