fix: extension command with quotes in cli#8150
Conversation
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 80a0db6afb
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
crates/goose-cli/src/session/mod.rs
Outdated
| let mut parts: Vec<String> = shlex::split(extension_command) | ||
| .ok_or_else(|| anyhow::anyhow!("Invalid shell quoting in extension command"))?; |
There was a problem hiding this comment.
Using shlex::split here changes command semantics by applying POSIX escape rules, so unquoted backslashes are consumed. That breaks valid extension inputs like Windows executables (C:\tools\mcp.exe) or regex-style args (\d+) by turning them into C:toolsmcp.exe / d+, which then fails at runtime even though those strings previously worked with split_whitespace. Because this path parses --with-extension and /extension command text, the regression affects real user commands on Windows and any command needing literal backslashes.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
yes, shlex does not work for window. i have created an util for that now
lifeizhou-ap
requested review from
DOsinga,
angiejones and
blackgirlbytes
as code owners
|
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 107b93afb0
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| let shell_parts = shlex::split(&command_str) | ||
| .ok_or_else(|| anyhow::anyhow!("Invalid shell quoting in command"))?; |
There was a problem hiding this comment.
Using shlex::split here changes the raw command text before it is persisted, so unquoted backslashes are treated as escapes and removed. In goose configure, this breaks valid inputs like Windows executables (C:\tools\mcp.exe) or args that rely on literal backslashes, causing the saved cmd/args to differ from what the user entered and fail at runtime when launching the extension.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: af35eb6e8c
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| '"' if !in_single_quote => in_double_quote = !in_double_quote, | ||
| '\'' if !in_double_quote => in_single_quote = !in_single_quote, |
There was a problem hiding this comment.
split_quoted flips quote state on every "/' character, even when the quote is meant to be literal content inside an unquoted token. That corrupts valid extension arguments like --json {"a":"b"} into {a:b} and can reject paths such as C:\Users\O'Connor\mcp.exe with Unmatched quote. This is a regression from the previous whitespace split behavior and affects all stdio extension entry points (--with-extension, /extension, and goose configure) that call parse_stdio_extension/split_quoted.
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
These are the edge cases. we can address them if these issues are reported
* origin/main: (63 commits) remove name from blog post (#8157) fix: use `overflow: clip` to not disrupt sticky ToC (#8158) chore(deps): bump path-to-regexp from 0.1.12 to 0.1.13 in /documentation (#8161) chore(deps): bump node-forge from 1.3.2 to 1.4.0 in /documentation (#8145) refactor: goose-acp-server -> goose binary for TUI (#8155) fix "View as Markdown" feature not working (#8160) feat(tui): UI improvements for messages, tool calls, text entry, etc (#8156) feat(desktop): add i18n infrastructure with react-intl (#8105) fix(tui): ordering of messages (#8144) fix: extension command with quotes in cli (#8150) chore(aaif): Use Azure Artifact Signing for Windows (#8116) chore(aaif): Switch macOS code signing (#8076) Remove unused tool call json in logs (#8147) feat(tui): tab expand tool calls cleanly (#8136) fix: replace panics with user-friendly errors in CLI session builder (#7901) fix: read GOOSE_CONTEXT_LIMIT from config.yaml, not just env vars (#7900) fix: deliver truncation notice as separate content block (#7899) fix: use platform-appropriate commands in developer extension instructions (#7898) fix: replace any with proper SVG types in icon components (#7873) chore: remove debug console.log statements, stale comments, and dead code (#8142) ...
Signed-off-by: Cameron Yick <[email protected]>
2 participants
Summary
Fix Issue #6487