-
Notifications
You must be signed in to change notification settings - Fork 37.2k
Use unified timeout setting under shell integration in runCommand #272369
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Conversation
src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/toolTerminalCreator.ts
Outdated
Show resolved
Hide resolved
...rkbench/contrib/terminalContrib/chatAgentTools/common/terminalChatAgentToolsConfiguration.ts
Outdated
Show resolved
Hide resolved
There was a problem hiding this 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 unifies shell integration timeout configuration by introducing a new general terminal setting terminal.integrated.shellIntegration.timeout that replaces the chat-specific setting. The change allows this timeout to be used more broadly across terminal features, including the runCommand API.
Key Changes:
- Adds new unified
terminal.integrated.shellIntegration.timeoutsetting - Updates chat agent tools to prefer the new setting with fallback to deprecated setting
- Modifies
runCommandto use the unified timeout with adjusted wait logic based on process ready time
Reviewed Changes
Copilot reviewed 8 out of 8 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
src/vs/platform/terminal/common/terminal.ts |
Adds ShellIntegrationTimeout enum value for the new unified setting |
src/vs/workbench/contrib/terminal/common/terminalConfiguration.ts |
Defines the new terminal.integrated.shellIntegration.timeout configuration property |
src/vs/workbench/contrib/terminalContrib/chatAgentTools/common/terminalChatAgentToolsConfiguration.ts |
Marks the chat-specific timeout setting as deprecated and references the new unified setting |
src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/toolTerminalCreator.ts |
Updates to prefer the new unified timeout setting with fallback to deprecated chat setting |
src/vs/workbench/contrib/terminal/common/terminal.ts |
Adds processReadyTime property to track when terminal process becomes ready |
src/vs/workbench/contrib/terminal/browser/terminalProcessManager.ts |
Implements tracking of process ready time |
src/vs/workbench/contrib/terminal/browser/terminalInstance.ts |
Refactors runCommand to use unified timeout with elapsed time calculation |
src/vs/workbench/contrib/terminal/browser/terminalCommands.ts |
Adds temporary test command for validating the runCommand timeout behavior |
...rkbench/contrib/terminalContrib/chatAgentTools/common/terminalChatAgentToolsConfiguration.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/toolTerminalCreator.ts
Outdated
Show resolved
Hide resolved
...rkbench/contrib/terminalContrib/chatAgentTools/common/terminalChatAgentToolsConfiguration.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/toolTerminalCreator.ts
Outdated
Show resolved
Hide resolved
...rkbench/contrib/terminalContrib/chatAgentTools/common/terminalChatAgentToolsConfiguration.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/toolTerminalCreator.ts
Outdated
Show resolved
Hide resolved
…ering processReadytime
src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/toolTerminalCreator.ts
Show resolved
Hide resolved
src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/toolTerminalCreator.ts
Show resolved
Hide resolved
...bench/contrib/terminalContrib/chatAgentTools/browser/terminal.chatAgentTools.contribution.ts
Outdated
Show resolved
Hide resolved
...bench/contrib/terminalContrib/chatAgentTools/browser/terminal.chatAgentTools.contribution.ts
Outdated
Show resolved
Hide resolved
src/vs/workbench/contrib/terminal/common/terminalConfiguration.ts
Outdated
Show resolved
Hide resolved
Co-authored-by: Daniel Imms <[email protected]>
Tyriar
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Red CI
platform should be small, it's not used there
Resolves: microsoft/vscode#273017 * Coming from: microsoft/vscode#272369 * Further ensure so that pwsh command activation does not hang. * Took the regex from: https://github.com/microsoft/vscode/blob/e1f78f13a11e6a1f20dfacbe03f8c7b64b031ac2/src/vs/workbench/contrib/terminalContrib/chatAgentTools/browser/executeStrategy/executeStrategy.ts#L53-L97 * Already updated distro to enable onTerminalDataWrite proposed api. --------- Co-authored-by: Daniel Imms <[email protected]>
Resolves: #270813
Main points from: #272369 (comment)