Skip to content

Comments

feat(shell): enhance shell tool with version-aware PowerShell context#1135

Open
QIN2DIM wants to merge 7 commits intoMoonshotAI:mainfrom
QIN2DIM:improve-shell-tool
Open

feat(shell): enhance shell tool with version-aware PowerShell context#1135
QIN2DIM wants to merge 7 commits intoMoonshotAI:mainfrom
QIN2DIM:improve-shell-tool

Conversation

@QIN2DIM
Copy link
Contributor

@QIN2DIM QIN2DIM commented Feb 13, 2026

Related Issue

Resolve #1136

Description

Shell: Add version-aware documentation that detects PowerShell 5.1 vs 7+ and provides appropriate syntax guidance

# Testing Scenario: Model Requires Continuous Execution of Multiple Commands
TEST_SCENARIOS = [
    {
        "name": "连续执行命令(无条件)",
        "task": "创建目录 test,进入 test,创建文件 file.txt",
        "bad_pattern": "mkdir test && cd test && touch file.txt",  # PS 5.1 不支持
        "good_ps5": "mkdir test; Set-Location test; New-Item file.txt",
        "good_ps7": "mkdir test && cd test && New-Item file.txt",
    },
    {
        "name": "条件执行(成功时)",
        "task": "运行 python script.py,成功则输出 Success",
        "bad_pattern": "python script.py && echo Success",
        "good_ps5": "python script.py; if ($?) { echo Success }",
        "good_ps7": "python script.py && echo Success",
    },
    {
        "name": "条件执行(失败时)",
        "task": "运行 python script.py,失败则输出 Failed",
        "bad_pattern": "python script.py || echo Failed",
        "good_ps5": "python script.py; if ($LASTEXITCODE -ne 0) { echo Failed }",
        "good_ps7": "python script.py || echo Failed",
    },
]

Checklist

  • I have read the CONTRIBUTING document.
  • I have linked the related issue, if any.
  • I have added tests that prove my fix is effective or that my feature works.
  • I have run make gen-changelog to update the changelog.
  • I have run make gen-docs to update the user documentation.

Open with Devin

This commit improves the shell tool by introducing version-specific documentation for PowerShell environments.

- Added dynamic shell key detection in `Shell._get_shell_key()` to distinguish between:
  - `bash` for non-Windows systems
  - `powershell5` for Windows PowerShell 5.1
  - `powershell7` for PowerShell 7+

- Created new documentation files:
  - `powershell5.md`: Specific guidelines for PowerShell 5.1 with syntax limitations and required workarounds
  - `powershell7.md`: Modern PowerShell 7+ features including `&&`/`||` operators and native cmdlet usage

- Removed obsolete `powershell.md` file and updated environment detection to include shell version detection

- Enhanced `Environment` class with async shell version detection via `_get_powershell_version()` and `_get_bash_version()`

- Updated shell description to include detected shell version in the template context

This change ensures users receive accurate, version-specific guidance, improving both safety and efficiency when using the shell tool on different Windows PowerShell versions.
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 potential issue.

View 5 additional findings in Devin Review.

Open in Devin Review

@QIN2DIM QIN2DIM changed the title Improve shell tool feat(tool/shell): Improve shell tool powershell Feb 13, 2026
@QIN2DIM QIN2DIM changed the title feat(tool/shell): Improve shell tool powershell feat(shell): enhance shell tool with version-aware PowerShell context Feb 13, 2026
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

View 8 additional findings in Devin Review.

Open in Devin Review

Co-authored-by: devin-ai-integration[bot] <158243242+devin-ai-integration[bot]@users.noreply.github.com>
Signed-off-by: QIN2DIM <62018067+QIN2DIM@users.noreply.github.com>
Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

View 10 additional findings in Devin Review.

Open in Devin Review

Copy link
Contributor

@devin-ai-integration devin-ai-integration bot left a comment

Choose a reason for hiding this comment

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

Devin Review found 1 new potential issue.

View 11 additional findings in Devin Review.

Open in Devin Review

stdout=asyncio.subprocess.PIPE,
stderr=asyncio.subprocess.PIPE,
)
stdout, _ = await asyncio.wait_for(proc.communicate(), timeout=2.0)
Copy link
Contributor

Choose a reason for hiding this comment

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

🟡 Process resource leak in _get_bash_version on timeout

In _get_bash_version, if asyncio.wait_for raises TimeoutError, the spawned bash process is never killed. The exception is caught by the broad except Exception handler which returns "unknown" but leaves the child process running.

Root Cause and Comparison with _get_powershell_version

In _get_powershell_version (src/kimi_cli/utils/environment.py:106-110), there is an explicit inner try/except asyncio.TimeoutError block that properly cleans up:

try:
    stdout, _ = await asyncio.wait_for(proc.communicate(), timeout=3.0)
except asyncio.TimeoutError:
    proc.kill()
    await proc.wait()
    return "unknown"

But in _get_bash_version, no such cleanup exists:

proc = await asyncio.create_subprocess_exec(bash_path, "--version", ...)
stdout, _ = await asyncio.wait_for(proc.communicate(), timeout=2.0)
# If TimeoutError is raised here, proc is never killed

The TimeoutError (which is asyncio.TimeoutError in Python 3.11+) propagates to the outer except Exception on line 142, which returns "unknown" without terminating the child process. This leaves a zombie/orphaned process.

Impact: While bash --version is unlikely to hang in practice, this is an inconsistency with the sibling function and a potential process leak.

Suggested change
stdout, _ = await asyncio.wait_for(proc.communicate(), timeout=2.0)
try:
stdout, _ = await asyncio.wait_for(proc.communicate(), timeout=2.0)
except asyncio.TimeoutError:
proc.kill()
await proc.wait()
return "unknown"
Open in Devin Review

Was this helpful? React with 👍 or 👎 to provide feedback.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat(shell): enhance shell tool with version-aware PowerShell context

1 participant