feat(cmd): add new_direct and raw_arg for Windows quoting by jdx · Pull Request #89 · jdx/ensembler

jdx · 2026-04-12T12:05:09Z

Summary

Add CmdLineRunner::new_direct which skips the Windows auto cmd.exe /c wrapping so callers can construct cmd.exe invocations precisely.
Add CmdLineRunner::raw_arg which appends a fragment verbatim via CommandExt::raw_arg, bypassing Rust's MSVCRT-style re-escaping. Falls back to arg on non-Windows.

Why

On Windows, Command::arg applies MSVCRT argv escaping which conflicts with cmd.exe's own quoting rules. Strings that are already shell-quoted for cmd.exe (e.g. rendered {{files}} values in hk) get mangled — cmd.exe sees literal " characters in tool arguments, which silently corrupts file paths passed to linters. See jdx/hk#823 for the downstream bug.

Test plan

Consumed by hk; regression covered by a new Pester e2e test in that repo.
Cross-compile ensembler for x86_64-pc-windows-gnu passes.

🤖 Generated with Claude Code

Note

Medium Risk
Medium risk because it changes how Windows command lines can be constructed and introduces a raw_arg path that bypasses normal argument escaping, which could affect command execution/quoting behavior if misused.

Overview
Adds opt-in APIs to better control Windows command invocation and quoting in CmdLineRunner.

CmdLineRunner::new_direct allows bypassing the default Windows cmd.exe /c wrapper, and CmdLineRunner::raw_arg uses CommandExt::raw_arg on Windows to append unescaped fragments (falling back to arg elsewhere), avoiding double-escaping for already shell-quoted payloads; construction is refactored through a shared init helper.

^{Reviewed by Cursor Bugbot for commit d4ab94b. Bugbot is set up for automated code reviews on this repo. Configure here.}

gemini-code-assist

Code Review

This pull request refactors CmdLineRunner by introducing a private init method and adding new_direct and raw_arg methods to provide better control over command execution, particularly on Windows. Feedback suggests improving the raw_arg implementation by using the original OsStr on Windows to avoid potential data loss from lossy string conversion and removing an unnecessary unused_mut attribute.

gemini-code-assist · 2026-04-12T12:07:19Z

+    #[allow(unused_mut)]
+    pub fn raw_arg<S: AsRef<OsStr>>(mut self, arg: S) -> Self {
+        let s = arg.as_ref().to_string_lossy().to_string();
+        #[cfg(windows)]
+        {
+            use std::os::windows::process::CommandExt;
+            self.cmd.as_std_mut().raw_arg(&s);
+        }
+        #[cfg(not(windows))]
+        {
+            self.cmd.arg(arg.as_ref());
+        }
+        self.args.push(s);


The implementation of raw_arg can be improved for consistency and efficiency.

Consistency & Correctness: On Windows, the current code passes a lossy String (&s) to raw_arg, while on other platforms it passes the original OsStr. Using the original OsStr on Windows avoids potential data loss if the input contains non-UTF-8 sequences that are valid in the system encoding.

Redundant Attribute: The #[allow(unused_mut)] attribute is unnecessary because self is mutated when pushing to self.args and when calling as_std_mut() or arg() on the command.

Efficiency: We can avoid creating the lossy string until it's actually needed for the self.args vector.

Suggested change

#[allow(unused_mut)]

pub fn raw_arg<S: AsRef<OsStr>>(mut self, arg: S) -> Self {

let s = arg.as_ref().to_string_lossy().to_string();

#[cfg(windows)]

{

use std::os::windows::process::CommandExt;

self.cmd.as_std_mut().raw_arg(&s);

}

#[cfg(not(windows))]

{

self.cmd.arg(arg.as_ref());

}

self.args.push(s);

pub fn raw_arg<S: AsRef<OsStr>>(mut self, arg: S) -> Self {

let arg = arg.as_ref();

#[cfg(windows)]

{

use std::os::windows::process::CommandExt;

self.cmd.as_std_mut().raw_arg(arg);

}

#[cfg(not(windows))]

{

self.cmd.arg(arg);

}

self.args.push(arg.to_string_lossy().to_string());

greptile-apps · 2026-04-12T12:08:12Z

Greptile Summary

Adds CmdLineRunner::new_direct (direct program invocation, skipping the Windows cmd.exe /c auto-wrap) and CmdLineRunner::raw_arg (verbatim command-line fragment via CommandExt::raw_arg on Windows, falling back to arg elsewhere), and cleanly refactors the shared constructor boilerplate into a private init helper. All logic is correct and the Windows/non-Windows conditional paths are properly symmetric.

Confidence Score: 5/5

Safe to merge; only one minor unnecessary lint-suppression attribute found, no functional issues.

All changes are correct: the init refactor is a pure mechanical extraction, new_direct is a straightforward Command::new wrapper, and raw_arg correctly routes to CommandExt::raw_arg on Windows via as_std_mut() and falls back to the regular arg path on non-Windows. The single finding is a dead #[allow(unused_mut)] attribute — the mut is genuinely needed in both cfg paths and the lint would never fire, so the allow is a no-op rather than a real concern.

No files require special attention.

Important Files Changed

Filename	Overview
src/cmd.rs	Adds `new_direct` constructor (bypasses Windows cmd.exe wrap), refactors shared init into a private `init` helper, and adds `raw_arg` for verbatim Windows command-line fragments. Logic is correct; a minor dead lint suppression exists on `raw_arg`.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[CmdLineRunner constructor] --> B{Which constructor?}
    B -->|new| C{Windows?}
    B -->|new_direct| D[Command::new program]
    C -->|Yes| E[cmd.exe /c program]
    C -->|No| D
    E --> F[init: set stdin/stdout/stderr pipes]
    D --> F
    F --> G[CmdLineRunner ready]

    G --> H{Add argument}
    H -->|.arg| I[Command::arg MSVCRT-escaped]
    H -->|.raw_arg| J{Windows?}
    J -->|Yes| K[as_std_mut.raw_arg verbatim]
    J -->|No| I
    I --> L[self.args.push for display/errors]
    K --> L

_{Reviews (2): Last reviewed commit: "feat(cmd): add new_direct and raw_arg fo..." | Re-trigger Greptile}

cursor

Cursor Bugbot has reviewed your changes and found 1 potential issue.

^{❌ Bugbot Autofix is OFF. To automatically fix reported issues with cloud agents, enable autofix in the Cursor dashboard.}

^{Reviewed by Cursor Bugbot for commit 602f2d1. Configure here.}

cursor · 2026-04-12T12:10:30Z

+        #[cfg(windows)]
+        {
+            use std::os::windows::process::CommandExt;
+            self.cmd.as_std_mut().raw_arg(&s);


Lossy conversion loses data before passing to raw_arg

Low Severity

On Windows, raw_arg eagerly converts the input OsStr to a String via to_string_lossy() on line 201 and then passes that lossy string to CommandExt::raw_arg on line 205. This contrasts with arg, which passes the original OsStr directly to self.cmd.arg(arg.as_ref()) and only uses the lossy conversion for the display-only self.args vec. If the input contains non-UTF-8 sequences (e.g. WTF-8 unpaired surrogates in a Windows path), raw_arg silently replaces them with U+FFFD, losing data. Passing arg.as_ref() directly to raw_arg instead of &s would preserve fidelity and be consistent with arg.

^{Reviewed by Cursor Bugbot for commit 602f2d1. Configure here.}

On Windows, Rust's `Command::arg` applies MSVCRT-style argv escaping, which collides with cmd.exe's own quoting rules and mangles strings that already contain cmd-appropriate quoting (e.g. rendered `{{files}}` values in hk). Add two APIs: - `CmdLineRunner::new_direct` skips the auto `cmd.exe /c` wrapping performed by `new` on Windows, letting callers construct cmd.exe invocations precisely. - `CmdLineRunner::raw_arg` appends a fragment verbatim to the command line via `CommandExt::raw_arg`, bypassing Rust's re-escaping so shell-quoted payloads reach cmd.exe intact. Falls back to `arg` on non-Windows. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

Fixes #823. ## Summary - On Windows, Rust's `Command::arg` applies MSVCRT-style argv escaping that collides with `cmd.exe`'s own quoting, so a rendered `{{files}}` payload (already quoted via `ShellType::Cmd::quote`) reaches tools with literal `"` characters in the arguments. In practice this silently broke `ruff`, `biome`, and any other `{{files}}`-based check — they'd exit 0 while processing zero files, so hk would report success on malformed invocations. - Switch the Windows `cmd.exe /c` path in `src/step/runner.rs` to use a new `CmdLineRunner::raw_arg` (paired with `new_direct` to avoid the implicit `cmd.exe /c` wrapping), so the rendered command string is appended verbatim via `CommandExt::raw_arg` and cmd.exe can parse its own quoting without Rust interference. Also covers `{{workspace_files}}`, which goes through the same shell-quoting path. - Requires ensembler PR: jdx/ensembler#89 (submodule pointer bumped in this PR). ## Test plan - [x] `cargo check` (linux) passes. - [x] `cargo check --target x86_64-pc-windows-gnu` passes. - [x] Added a Pester regression test `{{files}} template passes clean arguments on Windows (no literal quotes)` in `e2e-win/check.Tests.ps1`: materializes files including `hello world.txt`, renders `python check_args.py {{files}}`, and fails if any arg contains a literal `"` or references a missing path. - [x] Wired the new Pester test into the `ci-windows` job in `.github/workflows/ci.yml`. 🤖 Generated with [Claude Code](https://claude.com/claude-code)  --- > [!NOTE] > **Medium Risk** > Changes how step commands are executed on Windows `cmd.exe` by switching to a raw-argument path; this can affect argument parsing/escaping and could alter behavior for existing Windows hooks. > > **Overview** > Fixes Windows `cmd.exe` execution so rendered `{{files}}`/`{{workspace_files}}` strings are passed *verbatim* to the shell (using `CmdLineRunner::new_direct` + `raw_arg`) instead of being re-escaped by Rust, preventing literal quote characters from reaching tools and silently dropping files. > > Adds a Windows Pester regression test that validates `{{files}}` expands to real paths (including spaces) without embedded quotes, and wires that test into the `ci-windows` GitHub Actions job. > > <sup>Reviewed by [Cursor Bugbot](https://cursor.com/bugbot) for commit 6693338. Bugbot is set up for automated code reviews on this repo. Configure [here](https://www.cursor.com/dashboard/bugbot).</sup>  --------- Co-authored-by: Claude Opus 4.6 (1M context) <noreply@anthropic.com>

jdx mentioned this pull request Apr 12, 2026

fix(step): preserve cmd.exe quoting for {{files}} on Windows jdx/hk#824

Merged

4 tasks

gemini-code-assist Bot reviewed Apr 12, 2026

View reviewed changes

cursor Bot reviewed Apr 12, 2026

View reviewed changes

jdx force-pushed the fix/windows-raw-arg branch from 602f2d1 to d4ab94b Compare April 12, 2026 12:11

jdx merged commit e50b3ee into main Apr 12, 2026
9 checks passed

jdx deleted the fix/windows-raw-arg branch April 12, 2026 12:15

jdx mentioned this pull request Apr 12, 2026

chore: release v1.1.0 #90

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat(cmd): add new_direct and raw_arg for Windows quoting#89

feat(cmd): add new_direct and raw_arg for Windows quoting#89
jdx merged 1 commit intomainfrom
fix/windows-raw-arg

jdx commented Apr 12, 2026 •

edited by cursor Bot

Loading

Uh oh!

gemini-code-assist Bot left a comment

Uh oh!

gemini-code-assist Bot Apr 12, 2026

Uh oh!

greptile-apps Bot commented Apr 12, 2026 •

edited

Loading

Uh oh!

cursor Bot left a comment

Uh oh!

cursor Bot Apr 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

Conversation

jdx commented Apr 12, 2026 • edited by cursor Bot Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Why

Test plan

Uh oh!

gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

Code Review

Uh oh!

gemini-code-assist Bot Apr 12, 2026

Choose a reason for hiding this comment

Uh oh!

greptile-apps Bot commented Apr 12, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Greptile Summary

Confidence Score: 5/5

Important Files Changed

Flowchart

Uh oh!

cursor Bot left a comment

Choose a reason for hiding this comment

Uh oh!

cursor Bot Apr 12, 2026

Choose a reason for hiding this comment

Lossy conversion loses data before passing to raw_arg

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

jdx commented Apr 12, 2026 •

edited by cursor Bot

Loading

greptile-apps Bot commented Apr 12, 2026 •

edited

Loading

Lossy conversion loses data before passing to `raw_arg`