Skip to content

Fix task command line reported when term is reused#275683

Merged
Tyriar merged 1 commit intomainfrom
tyriar/272945
Nov 5, 2025
Merged

Fix task command line reported when term is reused#275683
Tyriar merged 1 commit intomainfrom
tyriar/272945

Conversation

@Tyriar
Copy link
Member

@Tyriar Tyriar commented Nov 5, 2025

Fixes #272945

@Tyriar Tyriar added this to the October 2025 milestone Nov 5, 2025
@Tyriar Tyriar requested a review from meganrogge November 5, 2025 21:20
@Tyriar Tyriar self-assigned this Nov 5, 2025
Copilot AI review requested due to automatic review settings November 5, 2025 21:20
@Tyriar Tyriar enabled auto-merge November 5, 2025 21:20
Copy link
Contributor

Copilot AI left a comment

Choose a reason for hiding this comment

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

Pull Request Overview

This PR adds tracking and persistence of the shellIntegrationNonce for task terminals to ensure proper shell integration trust when terminals are reused or reconnected. When a terminal is reused for a new task, the nonce in the command line sequence must match the terminal's existing nonce for commands to be trusted correctly.

Key changes:

  • Added shellIntegrationNonce field to ITerminalData and IReconnectionTaskData interfaces
  • Captured and stored the nonce when terminals are created and reconnected
  • Implemented nonce rewriting in initialText for reused terminals to maintain shell integration trust

// command line sequence reports the correct nonce and becomes trusted as a result.
if (terminalToReuse.shellIntegrationNonce) {
if (Types.isString(launchConfigs.initialText) && launchConfigs.shellIntegrationNonce) {
launchConfigs.initialText = launchConfigs.initialText.replace(launchConfigs.shellIntegrationNonce, terminalToReuse.shellIntegrationNonce);
Copy link

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

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

The replace call only replaces the first occurrence of the nonce string. If shellIntegrationNonce appears multiple times in initialText, only the first occurrence will be replaced. Consider using replaceAll instead to ensure all occurrences are replaced: launchConfigs.initialText = launchConfigs.initialText.replaceAll(launchConfigs.shellIntegrationNonce, terminalToReuse.shellIntegrationNonce);

Suggested change
launchConfigs.initialText = launchConfigs.initialText.replace(launchConfigs.shellIntegrationNonce, terminalToReuse.shellIntegrationNonce);
launchConfigs.initialText = launchConfigs.initialText.replaceAll(launchConfigs.shellIntegrationNonce, terminalToReuse.shellIntegrationNonce);

Copilot uses AI. Check for mistakes.
Comment on lines +1561 to +1562
}
}
Copy link

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

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

The nonce replacement logic doesn't handle the case where initialText is an object with { text: string; trailingNewLine: boolean } format. Based on lines 1320-1323 and 1363-1366, initialText can be either a string or an object. The replacement should also handle the object case: if (Types.isString(launchConfigs.initialText)) { ... } else if (launchConfigs.initialText && typeof launchConfigs.initialText === 'object') { launchConfigs.initialText.text = launchConfigs.initialText.text.replaceAll(...); }

Suggested change
}
}
} else if (
launchConfigs.initialText &&
typeof launchConfigs.initialText === 'object' &&
typeof launchConfigs.initialText.text === 'string' &&
launchConfigs.shellIntegrationNonce
) {
launchConfigs.initialText.text = launchConfigs.initialText.text.replace(launchConfigs.shellIntegrationNonce, terminalToReuse.shellIntegrationNonce);
}

Copilot uses AI. Check for mistakes.
@@ -1551,6 +1553,13 @@ export class TerminalTaskSystem extends Disposable implements ITaskSystem {
if (task.configurationProperties.isBackground) {
launchConfigs.reconnectionProperties = { ownerId: TaskTerminalType, data: { lastTask: task.getCommonTaskId(), group, label: task._label, id: task._id } };
Copy link

Copilot AI Nov 5, 2025

Choose a reason for hiding this comment

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

The shellIntegrationNonce is stored in the ITerminalData and IReconnectionTaskData interfaces, but it's not being persisted to reconnectionProperties.data at line 1554 and 1575. This means that when a background task terminal reconnects after a reload, the nonce won't be available. Consider adding shellIntegrationNonce to the reconnection data: data: { lastTask: task.getCommonTaskId(), group, label: task._label, id: task._id, shellIntegrationNonce: launchConfigs.shellIntegrationNonce }

Suggested change
launchConfigs.reconnectionProperties = { ownerId: TaskTerminalType, data: { lastTask: task.getCommonTaskId(), group, label: task._label, id: task._id } };
launchConfigs.reconnectionProperties = { ownerId: TaskTerminalType, data: { lastTask: task.getCommonTaskId(), group, label: task._label, id: task._id, shellIntegrationNonce: launchConfigs.shellIntegrationNonce } };

Copilot uses AI. Check for mistakes.
@Tyriar Tyriar merged commit 3b6478c into main Nov 5, 2025
33 of 34 checks passed
@Tyriar Tyriar deleted the tyriar/272945 branch November 5, 2025 21:39
@vs-code-engineering vs-code-engineering bot locked and limited conversation to collaborators Dec 20, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Tasks do not trigger onDidStartTerminalShellExecution

3 participants