Repository navigation
Python: preserve Windows backslashes and quotes in LocalShellTool shell paths - #10145
Johnny Wilson Dougherty (JohnnyWilson16) wants to merge 1 commit into
Conversation
|
Johnny Wilson Dougherty (@JohnnyWilson16) please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
1 similar comment
|
Johnny Wilson Dougherty (@JohnnyWilson16) please read the following Contributor License Agreement(CLA). If you agree with the CLA, please reply with the following information.
Contributor License AgreementContribution License AgreementThis Contribution License Agreement (“Agreement”) is agreed to by the party signing below (“You”),
|
There was a problem hiding this comment.
🟡 Changes recommended
The Windows shell parser drops literal quotes encoded as consecutive quotation marks, silently changing arguments supplied through shell= or AGENT_FRAMEWORK_SHELL.
1 open issue
What changed in this PR
This PR addresses #9819 by adding Windows-specific parsing for LocalShellTool shell overrides, preserving path backslashes while retaining POSIX parsing elsewhere.
Changes:
- Adds Windows tokenization for explicit and environment-based overrides.
- Updates PowerShell detection and adds regression tests for Windows paths, quoting, and empty arguments.
- Adds implementation plans for this fix and a separate tool-approval budget cleanup.
| File | Description |
|---|---|
python/packages/tools/tests/test_shell_resolve.py |
Adds Windows shell parsing regression tests. |
python/packages/tools/agent_framework_tools/shell/_resolve.py |
Adds platform-specific tokenization and normalizes paths for PowerShell detection. |
docs/superpowers/plans/2026-11-06-issue-9819-windows-shell-parsing.md |
Documents the Windows parsing implementation plan. |
docs/superpowers/plans/2026-11-06-issue-9677-tool-approval-budget-cleanup.md |
Adds an unrelated tool-approval budget cleanup plan; no runtime changes. |
Review effort: Balanced
| elif c == '"': | ||
| in_quotes = not in_quotes |
BlueRaddish
left a comment
There was a problem hiding this comment.
Here is a concrete follow-up patch for the existing consecutive-quote finding, based on 8b47e6664be404472a06f28f357375653808e57b.
It preserves a doubled quote while already inside quotes, in both the ordinary quote branch and the even-backslash-before-quote branch. The eight regression cases exercise the explicit shell and AGENT_FRAMEWORK_SHELL paths; the existing 16 owning cases are unchanged. This keeps the current backslash, empty-argument, PowerShell detection and POSIX behavior.
--- a/python/packages/tools/agent_framework_tools/shell/_resolve.py
+++ b/python/packages/tools/agent_framework_tools/shell/_resolve.py
@@ -45,16 +45,24 @@
has_token = True
i += 1
else:
- in_quotes = not in_quotes
+ if in_quotes and i + 1 < n and cmd[i + 1] == '"':
+ buf.append('"')
+ i += 2
+ else:
+ in_quotes = not in_quotes
+ i += 1
has_token = True
- i += 1
else:
buf.append("\\" * num_slashes)
has_token = True
elif c == '"':
- in_quotes = not in_quotes
+ if in_quotes and i + 1 < n and cmd[i + 1] == '"':
+ buf.append('"')
+ i += 2
+ else:
+ in_quotes = not in_quotes
+ i += 1
has_token = True
- i += 1
else:
buf.append(c)
has_token = True
--- a/python/packages/tools/tests/test_shell_resolve.py
+++ b/python/packages/tools/tests/test_shell_resolve.py
@@ -135,3 +135,24 @@
assert argv[0] == r"C:\Program Files\PowerShell\7\pwsh.exe"
assert argv[1] == "-NoProfile"
assert argv[-1] == "-Command"
+
+
+@pytest.mark.parametrize("from_env", [False, True], ids=["explicit", "env"])
+@pytest.mark.parametrize(
+ ("command_line", "expected"),
+ [
+ ('bash "a""b"', ["bash", 'a"b']),
+ ('bash "a"""', ["bash", 'a"']),
+ ('bash """a', ["bash", '"a']),
+ (r'bash "a\\""b"', ["bash", 'a\\"b']),
+ ],
+)
+def test_win32_consecutive_quotes(
+ command_line: str, expected: list[str], from_env: bool, monkeypatch: pytest.MonkeyPatch
+) -> None:
+ import agent_framework_tools.shell._resolve as resolve_module
+
+ monkeypatch.setattr(resolve_module.sys, "platform", "win32")
+ if from_env:
+ monkeypatch.setenv("AGENT_FRAMEWORK_SHELL", command_line)
+ assert resolve_shell(None if from_env else command_line, interactive=True) == expectedVerification on Windows 11 / Python 3.11.17 / pytest 9.1.1:
- Current complete resolver module with the final test file: 16 passed, 8 failed, 0 skipped. With this patch: 24 passed, 0 failed, 0 skipped.
- Compared 21 argument vectors to the actual Windows
CommandLineToArgvWAPI: current parser has 4 mismatches, patched parser has 0. Cases include empty arguments, escaped quotes, trailing backslashes, adjacent fragments and consecutive quotes. The oracle prepends a dummy executable to test ordinary argument parsing; it deliberately excludes Windows' special executable-token rule, preserving this PR's adjacent-fragment behavior. - The PR's declared Ruff 0.17.3 check and format check pass on the two proposed files under the exact root/package configuration.
Scope: the owning run executes the full resolver module and exact current test bodies with real pytest/MonkeyPatch and the exact ShellExecutionError class. Import-only package shims isolate unrelated SDK initialization. This does not claim full package initialization, repository-wide fixtures/suites, actual shell launch, native commit hooks or full workspace acceptance. The probes execute in their own verified, bounded processes; no model/provider calls were made.
AI assistance was used for implementation, regression tests and verification preparation. This is working implementation assistance for the already-reported gap, rather than a competing PR or a new report of that same finding.

Motivation & Context
On Windows, configuring
LocalShellTool(shell=r"C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe")or settingAGENT_FRAMEWORK_SHELLto an unquoted Windows path fails withFileNotFoundError: [WinError 2].resolve_shellused POSIXshlex.split, which strips backslashes as escape characters and fails to identify the binary as PowerShell.Prior PR #9858 was closed by maintainer Eduard van Valkenburg (@eavanvalkenburg) with a request for a fresh PR with a complete Windows command-line parser that preserves escaped quotes and explicitly quoted empty arguments without dropping tokens.
Description & Review Guide
_split_windows_command_lineinagent_framework_tools/shell/_resolve.pyimplementingCommandLineToArgvWrules:"").\") and consecutive backslash sequences preceding quotes (_split_shell_overrideto dispatch to_split_windows_command_lineonsys.platform == "win32"while preserving standardshlex.splitbehavior on POSIX systems.is_powershellto normalize backslashes beforeos.path.basenameso Windows executable paths evaluate correctly across platforms.test_shell_resolve.pycovering unquoted Windows paths, environment variable overrides, empty arguments, and adjacent quoted/unquoted tokens under win32 platform mocks.LocalShellToolandAGENT_FRAMEWORK_SHELL. POSIX behavior is unchanged._split_windows_command_lineto WindowsCommandLineToArgvWargument tokenization rules.Related Issue
Fixes #9819
AI Assistance
AI assistance details: Used an AI coding assistant to help draft the Windows tokenizer following CommandLineToArgvW rules, write the regression tests, and prepare this description.
Contribution Checklist