From ae5f930d8c5be07a15f65ae39799dc28fcd2d589 Mon Sep 17 00:00:00 2001 From: Lars Vogel Date: Wed, 14 Oct 2026 22:22:44 +0200 Subject: [PATCH] Complete running terminal command when its tab is closed MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closing the Copilot terminal tab while a foreground command was running left the command's future incomplete, so the agent's run_in_terminal call never returned and the chat turn hung until cancelled. When the disposed widget is the current Copilot tab, the dispose listener now clears the tab, view control and output buffer and completes the pending command. The listener takes no lock because it runs on the UI thread while lock holders wait on syncExec, so the shared fields are volatile, the pending command is swapped atomically and executeCommand and revealTerminal() work on snapshots. Assisted-by: multiple AI agents and layers of automated tooling 🤖 --- .../ui/terminal/tm/RunInTerminalTool.java | 45 +++++++++++-------- .../ui/terminal/RunInTerminalTool.java | 45 +++++++++++-------- 2 files changed, 52 insertions(+), 38 deletions(-) diff --git a/com.microsoft.copilot.eclipse.ui.terminal.tm/src/com/microsoft/copilot/eclipse/ui/terminal/tm/RunInTerminalTool.java b/com.microsoft.copilot.eclipse.ui.terminal.tm/src/com/microsoft/copilot/eclipse/ui/terminal/tm/RunInTerminalTool.java index 4e27a0d3..a1a5d21d 100644 --- a/com.microsoft.copilot.eclipse.ui.terminal.tm/src/com/microsoft/copilot/eclipse/ui/terminal/tm/RunInTerminalTool.java +++ b/com.microsoft.copilot.eclipse.ui.terminal.tm/src/com/microsoft/copilot/eclipse/ui/terminal/tm/RunInTerminalTool.java @@ -53,19 +53,20 @@ public class RunInTerminalTool implements IRunInTerminalTool { private static final String POWERSHELL_SCRIPT_ENV = "COPILOT_POWERSHELL_INTEGRATION_SCRIPT"; private static final String COMMAND_CANCELLED_MESSAGE = "Terminal command cancelled."; private static final String COMMAND_INTERRUPTED_MESSAGE = "Terminal command interrupted by a new command."; + private static final String TERMINAL_CLOSED_MESSAGE = "Terminal was closed before the command finished."; // Non-background terminal field - private ITerminalViewControl persistentTerminalViewControl; + private volatile ITerminalViewControl persistentTerminalViewControl; // Terminal UI-related fields private ITerminalControl terminalControl; private CTabFolder tabFolder; - private CTabItem copilotTabItem; + private volatile CTabItem copilotTabItem; private Supplier terminalIconSupplier; // Output and command state private StringBuilder sb; - private volatile ForegroundCommand foregroundCommand; + private final AtomicReference foregroundCommand = new AtomicReference<>(); /** * Constructor for RunInTerminalTool. @@ -89,7 +90,7 @@ public CompletableFuture executeCommand(String command, boolean isBackgr : new ForegroundCommand(commandFuture, hasShellIntegrationMarker()); if (commandState != null) { - foregroundCommand = commandState; + foregroundCommand.set(commandState); if (!commandState.useMarker()) { // Retain only the last line (prompt) in the output buffer if (!sb.isEmpty()) { @@ -108,9 +109,10 @@ public CompletableFuture executeCommand(String command, boolean isBackgr final String finalCommand = TerminalCommandProcessor.formatForExecution(command, useBracketedPaste()); synchronized (lock) { - if (!isBackground && this.persistentTerminalViewControl != null) { + ITerminalViewControl persistentControl = this.persistentTerminalViewControl; + if (!isBackground && persistentControl != null) { revealTerminal(); - sendCommand(this.persistentTerminalViewControl, finalCommand); + sendCommand(persistentControl, finalCommand); return commandFuture; } @@ -216,7 +218,7 @@ public void cancelCurrentCommand() { } private void closeRunningForegroundTerminal(String completionMessage) { - ForegroundCommand commandState = foregroundCommand; + ForegroundCommand commandState = foregroundCommand.get(); if (commandState != null && !commandState.future().isDone()) { closeCurrentForegroundTerminal(completionMessage); } @@ -226,8 +228,7 @@ private void closeCurrentForegroundTerminal(String completionMessage) { ForegroundCommand commandState = null; CTabItem tabItem = null; synchronized (lock) { - commandState = foregroundCommand; - foregroundCommand = null; + commandState = foregroundCommand.getAndSet(null); persistentTerminalViewControl = null; tabItem = copilotTabItem; copilotTabItem = null; @@ -249,8 +250,8 @@ private void closeCurrentForegroundTerminal(String completionMessage) { } private void clearForegroundCommand(ForegroundCommand commandState) { - if (commandState != null && foregroundCommand == commandState) { - foregroundCommand = null; + if (commandState != null) { + foregroundCommand.compareAndSet(commandState, null); } } @@ -346,7 +347,7 @@ private ITerminalServiceOutputStreamMonitorListener buildOutputStreamMonitorList output.append(content); // Detect completion based on platform strategy - ForegroundCommand commandState = foregroundCommand; + ForegroundCommand commandState = foregroundCommand.get(); if (!isBackground && commandState != null && !commandState.future().isDone()) { CompletionCheckResult completionResult = commandState.useMarker() ? TerminalCommandProcessor.tryCompleteWithMarker(output) @@ -360,9 +361,7 @@ private void handleCompletionResult(ForegroundCommand commandState, CompletionCh if (completionResult.state() == CompletionCheckState.INCOMPLETE) { return; } - if (foregroundCommand == commandState) { - foregroundCommand = null; - } + foregroundCommand.compareAndSet(commandState, null); if (!commandState.future().isDone()) { commandState.future().complete(completionResult.output()); } @@ -372,8 +371,15 @@ private DisposeListener buildDisposeListener(String executionId, boolean isBackg return e -> { if (isBackground) { backgroundCommandOutputs.remove(executionId); - } else { + } else if (e.widget == copilotTabItem) { + // No lock here: this runs on the UI thread and lock holders wait on syncExec. + copilotTabItem = null; persistentTerminalViewControl = null; + sb.setLength(0); + ForegroundCommand commandState = foregroundCommand.getAndSet(null); + if (commandState != null && !commandState.future().isDone()) { + commandState.future().complete(TERMINAL_CLOSED_MESSAGE); + } } if (backgroundCommandOutputs.isEmpty() && persistentTerminalViewControl == null) { @@ -383,7 +389,8 @@ private DisposeListener buildDisposeListener(String executionId, boolean isBackg } private void revealTerminal() { - if (tabFolder != null && copilotTabItem != null) { + CTabItem tabItem = copilotTabItem; + if (tabFolder != null && tabItem != null) { Display.getDefault().syncExec(() -> { try { IWorkbenchPage page = getActivePage(); @@ -393,8 +400,8 @@ private void revealTerminal() { tabFolder = view.getAdapter(CTabFolder.class); } } - if (tabFolder != null && !tabFolder.isDisposed()) { - tabFolder.setSelection(copilotTabItem); + if (tabFolder != null && !tabFolder.isDisposed() && !tabItem.isDisposed()) { + tabFolder.setSelection(tabItem); } } catch (PartInitException e) { // Skip exception diff --git a/com.microsoft.copilot.eclipse.ui.terminal/src/com/microsoft/copilot/eclipse/ui/terminal/RunInTerminalTool.java b/com.microsoft.copilot.eclipse.ui.terminal/src/com/microsoft/copilot/eclipse/ui/terminal/RunInTerminalTool.java index 0655e5c8..4d169637 100644 --- a/com.microsoft.copilot.eclipse.ui.terminal/src/com/microsoft/copilot/eclipse/ui/terminal/RunInTerminalTool.java +++ b/com.microsoft.copilot.eclipse.ui.terminal/src/com/microsoft/copilot/eclipse/ui/terminal/RunInTerminalTool.java @@ -53,19 +53,20 @@ public class RunInTerminalTool implements IRunInTerminalTool { private static final String POWERSHELL_SCRIPT_ENV = "COPILOT_POWERSHELL_INTEGRATION_SCRIPT"; private static final String COMMAND_CANCELLED_MESSAGE = "Terminal command cancelled."; private static final String COMMAND_INTERRUPTED_MESSAGE = "Terminal command interrupted by a new command."; + private static final String TERMINAL_CLOSED_MESSAGE = "Terminal was closed before the command finished."; // Non-background terminal field - private ITerminalViewControl persistentTerminalViewControl; + private volatile ITerminalViewControl persistentTerminalViewControl; // Terminal UI-related fields private ITerminalControl terminalControl; private CTabFolder tabFolder; - private CTabItem copilotTabItem; + private volatile CTabItem copilotTabItem; private Supplier terminalIconSupplier; // Output and command state private StringBuilder sb; - private volatile ForegroundCommand foregroundCommand; + private final AtomicReference foregroundCommand = new AtomicReference<>(); /** * Constructor for RunInTerminalTool. @@ -89,7 +90,7 @@ public CompletableFuture executeCommand(String command, boolean isBackgr : new ForegroundCommand(commandFuture, hasShellIntegrationMarker()); if (commandState != null) { - foregroundCommand = commandState; + foregroundCommand.set(commandState); if (!commandState.useMarker()) { // Retain only the last line (prompt) in the output buffer if (!sb.isEmpty()) { @@ -108,9 +109,10 @@ public CompletableFuture executeCommand(String command, boolean isBackgr final String finalCommand = TerminalCommandProcessor.formatForExecution(command, useBracketedPaste()); synchronized (lock) { - if (!isBackground && this.persistentTerminalViewControl != null) { + ITerminalViewControl persistentControl = this.persistentTerminalViewControl; + if (!isBackground && persistentControl != null) { revealTerminal(); - sendCommand(this.persistentTerminalViewControl, finalCommand); + sendCommand(persistentControl, finalCommand); return commandFuture; } @@ -217,7 +219,7 @@ public void cancelCurrentCommand() { } private void closeRunningForegroundTerminal(String completionMessage) { - ForegroundCommand commandState = foregroundCommand; + ForegroundCommand commandState = foregroundCommand.get(); if (commandState != null && !commandState.future().isDone()) { closeCurrentForegroundTerminal(completionMessage); } @@ -227,8 +229,7 @@ private void closeCurrentForegroundTerminal(String completionMessage) { ForegroundCommand commandState = null; CTabItem tabItem = null; synchronized (lock) { - commandState = foregroundCommand; - foregroundCommand = null; + commandState = foregroundCommand.getAndSet(null); persistentTerminalViewControl = null; tabItem = copilotTabItem; copilotTabItem = null; @@ -250,8 +251,8 @@ private void closeCurrentForegroundTerminal(String completionMessage) { } private void clearForegroundCommand(ForegroundCommand commandState) { - if (commandState != null && foregroundCommand == commandState) { - foregroundCommand = null; + if (commandState != null) { + foregroundCommand.compareAndSet(commandState, null); } } @@ -345,7 +346,7 @@ private ITerminalServiceOutputStreamMonitorListener buildOutputStreamMonitorList output.append(content); // Detect completion based on platform strategy - ForegroundCommand commandState = foregroundCommand; + ForegroundCommand commandState = foregroundCommand.get(); if (!isBackground && commandState != null && !commandState.future().isDone()) { CompletionCheckResult completionResult = commandState.useMarker() ? TerminalCommandProcessor.tryCompleteWithMarker(output) @@ -359,9 +360,7 @@ private void handleCompletionResult(ForegroundCommand commandState, CompletionCh if (completionResult.state() == CompletionCheckState.INCOMPLETE) { return; } - if (foregroundCommand == commandState) { - foregroundCommand = null; - } + foregroundCommand.compareAndSet(commandState, null); if (!commandState.future().isDone()) { commandState.future().complete(completionResult.output()); } @@ -371,8 +370,15 @@ private DisposeListener buildDisposeListener(String executionId, boolean isBackg return e -> { if (isBackground) { backgroundCommandOutputs.remove(executionId); - } else { + } else if (e.widget == copilotTabItem) { + // No lock here: this runs on the UI thread and lock holders wait on syncExec. + copilotTabItem = null; persistentTerminalViewControl = null; + sb.setLength(0); + ForegroundCommand commandState = foregroundCommand.getAndSet(null); + if (commandState != null && !commandState.future().isDone()) { + commandState.future().complete(TERMINAL_CLOSED_MESSAGE); + } } if (backgroundCommandOutputs.isEmpty() && persistentTerminalViewControl == null) { @@ -382,7 +388,8 @@ private DisposeListener buildDisposeListener(String executionId, boolean isBackg } private void revealTerminal() { - if (tabFolder != null && copilotTabItem != null) { + CTabItem tabItem = copilotTabItem; + if (tabFolder != null && tabItem != null) { Display.getDefault().syncExec(() -> { try { IWorkbenchPage page = getActivePage(); @@ -392,8 +399,8 @@ private void revealTerminal() { tabFolder = view.getAdapter(CTabFolder.class); } } - if (tabFolder != null && !tabFolder.isDisposed()) { - tabFolder.setSelection(copilotTabItem); + if (tabFolder != null && !tabFolder.isDisposed() && !tabItem.isDisposed()) { + tabFolder.setSelection(tabItem); } } catch (PartInitException e) { // Skip exception