fix(core): send shell output to model on cancel#20501
Conversation
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request addresses an issue where shell command output was lost upon cancellation. It introduces changes across the tool execution and state management layers to ensure that any partial output generated by a shell command before it is cancelled is captured and made available to the model. This significantly improves the user experience by providing valuable context even when operations are interrupted. Highlights
Changelog
Activity
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for Github and other Google products, sign up here. You can also get AI-powered code generation, chat, as well as code reviews directly in the IDE at no cost with the Gemini Code Assist IDE Extension. Footnotes
|
There was a problem hiding this comment.
Code Review
This pull request effectively addresses an issue where shell output was lost upon cancellation, ensuring partial output from the shell tool is propagated through the tool-executor and scheduler layers to the model for context. While the implementation includes robust type-safe handling of cancellation data, comprehensive new tests, and improved logic in shell.ts, a critical security vulnerability was identified. The ToolExecutor's newly added cancelled result path lacks the output truncation logic present in the success path. This omission could lead to memory exhaustion and Denial of Service if a cancelled command produces a large volume of output. It is highly recommended to apply consistent output truncation across all terminal tool states to address this security concern.
|
Size Change: +2.57 kB (+0.01%) Total Size: 25.9 MB
ℹ️ View Unchanged
|
|
Thanks for this PR! The implementation for sending partial shell output back to the model upon cancellation is solid. Extracting the output truncation logic so it applies uniformly is a great approach. I've reviewed the code against our project guidelines and have a few suggestions to improve maintainability and consistency: Improvements1. Code Duplication of Type Guard 2. Handling of resultDisplay:
typeof toolResult?.returnDisplay === 'string'
? toolResult.returnDisplay
: undefined,While this works perfectly for the shell tool (which produces a string upon cancellation), if another tool that returns a complex Nitpicks / Observations3. Type Assertions in Tests Overall, great work on fixing this issue! |

Summary
The shell output was getting dropped when users canceled before the shell finished running. There will be a follow-up PR to show the cancelled output in the UI.
Details
Passed the partial shell output from
shell.tsall the way up tostate-manager.ts.Related Issues
Fixes #19512
How to Validate
Pre-Merge Checklist