fix(plugins): use cmd.exe on Windows for shell hooks

[jmikedupont2]

Shell hooks fail on Windows because sh -c isn't available. This uses cmd.exe /C on Windows targets.

Changes

• Detect Windows at runtime and use cmd.exe /C instead of sh -c for shell hook execution

Testing

• Tested on Windows 10 with moltis v0.9.10 (port 57553)
• Windows CI passes

[greptile-apps]

Greptile Summary

This PR adds Windows support to ShellHookHandler by branching on cfg!(target_os = "windows") at compile time and invoking cmd.exe /C in place of sh -c. The production-code change is small, well-scoped, and structurally correct.

However, the test suite was not updated to reflect the new Windows code path:

• All tests in mod tests use bash/Unix shell syntax (>&2 redirections, $(...) sub-shells, sleep, grep, cut, etc.) that will not work when dispatched through cmd.exe /C.
• Without #[cfg(unix)] / #[cfg(windows)] guards on those tests, running the test suite on a Windows host will cause most tests to fail, leaving the new Windows path effectively untested.
• Command::new("cmd") is functional but "cmd.exe" (with extension) is the Windows convention and is slightly more defensive against shadowing.

Confidence Score: 3/5

• Safe to merge for non-Windows targets; Windows path is untested due to test suite not being updated.
• The one-line production change is correct and well-structured. The primary concern is that the test suite was not updated: all existing tests use bash/Unix syntax and will fail if executed on a Windows host, meaning the Windows code path has no real test coverage. Additionally, Command::new("cmd") is conventional but slightly less defensive than using "cmd.exe".
• crates/plugins/src/shell_hook.rs — specifically the test module, which needs #[cfg(unix)] guards and new Windows-specific tests.

Important Files Changed

Filename	Overview
crates/plugins/src/shell_hook.rs	Adds compile-time Windows detection to use cmd.exe /C instead of sh -c; the production logic change is minimal and correct, but the test suite was not updated and will fail on Windows because every existing test command uses Unix-only shell syntax.

Flowchart

%%{init: {'theme': 'neutral'}}%%
flowchart TD
    A[ShellHookHandler::handle called] --> B[Serialize HookPayload to JSON]
    B --> C{cfg! target_os = windows?}
    C -- Windows --> D[Command::new cmd.exe]
    D --> E[arg /C, arg command]
    C -- Unix/Other --> F[Command::new sh]
    F --> G[arg -c, arg command]
    E --> H[Set envs, stdin/stdout/stderr piped]
    G --> H
    H --> I{working_dir set?}
    I -- Yes --> J[cmd.current_dir]
    I -- No --> K[Spawn child process]
    J --> K
    K --> L[Write JSON payload to stdin]
    L --> M[Wait with timeout]
    M -- Timeout --> N[Return Err timeout]
    M -- exit 1 --> O[Return Ok Block reason]
    M -- exit != 0 --> P[Return Err non-zero exit]
    M -- exit 0 + empty stdout --> Q[Return Ok Continue]
    M -- exit 0 + JSON stdout --> R{Parse ShellHookResponse}
    R -- action = modify --> S[Return Ok ModifyPayload]
    R -- other / parse error --> Q

Loading

Comments Outside Diff (1)

1. crates/plugins/src/shell_hook.rs, line 217-433 (link)

   Tests use Unix-only shell syntax — will fail on Windows

   The PR correctly switches to cmd.exe /C on Windows, but all existing tests pass commands written in bash/Unix shell syntax. When the test suite runs on a Windows target, the code now dispatches to cmd.exe /C, and every one of these commands will either error or silently misbehave:

   • shell_hook_block_on_exit_one (line 235): "echo 'blocked by policy' >&2; exit 1" — >&2 is a bash-ism; cmd uses 1>&2
   • shell_hook_receives_payload_on_stdin (line 274): uses $(cat), grep, cut sub-shells — all Unix-only
   • shell_hook_timeout (line 295): "sleep 60" — sleep is not a cmd.exe built-in (use timeout /T 60)
   • shell_hook_env_vars (line 315): uses $VAR expansion syntax — cmd.exe requires %VAR%
   • shell_hook_working_dir (line 370): uses $(pwd) — cmd.exe has no sub-shell syntax

   Without platform-gating these tests, the CI report of "Windows CI passes" would only be reliable if the test suite itself was run on Windows. At minimum, Unix-specific tests should be annotated with #[cfg(unix)], and a corresponding #[cfg(windows)] set using cmd.exe-compatible commands should be added to provide real coverage of the new Windows path.

_{Reviews (1): Last reviewed commit: "fix(plugins): use cmd.exe on Windows for..." | Re-trigger Greptile}

[greptile-apps]

cmd.exe invoked without full path — prefer explicit binary name

Command::new("cmd") relies on cmd being resolvable via PATH. While cmd.exe is normally on the PATH on all Windows installations, using "cmd.exe" (with the .exe extension) is the Windows convention and avoids any edge-case ambiguity where a user might have a cmd script or binary earlier in their PATH.

Suggested change

	let mut cmd = if cfg!(target_os = "windows") {
	let mut c = Command::new("cmd");
	c.arg("/C").arg(&self.command);
	c
	} else {
	let mut c = Command::new("sh");
	c.arg("-c").arg(&self.command);
	c
	};
	let mut cmd = if cfg!(target_os = "windows") {
	let mut c = Command::new("cmd.exe");
	c.arg("/C").arg(&self.command);
	c
	} else {
	let mut c = Command::new("sh");
	c.arg("-c").arg(&self.command);
	c
	};

_{Note: If this suggestion doesn't match your team's coding style, reply to this and let me know. I'll remember it for next time!}