feat:LocalFilesystemWithShell.execute uses cmd /c to support Windows

[jinkunaier]

AgentScope-Java 2.x

Description

在LocalFilesystemWithShell execute方法中判断操作系统，通过 cmd /c 方式，支持在windows系统中执行脚本命令。

[CLAassistant]

Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.

---

jinkun seems not to be a GitHub user. You need a GitHub account to be able to sign the CLA. If you have already a GitHub account, please add the email address used for this commit to your account.
_{You have signed the CLA already but the status is still pending? Let us recheck it.}

[codecov]

Codecov Report

❌ Patch coverage is 55.55556% with 4 lines in your changes missing coverage. Please review.

Files with missing lines	Patch %	Lines
...ent/filesystem/local/LocalFilesystemWithShell.java	55.55%	3 Missing and 1 partial ⚠️

📢 Thoughts on this report? Let us know!

[AgentScopeJavaBot]

🤖 AI Review

This PR adds Windows support to LocalFilesystemWithShell.execute() by detecting the OS and using cmd /c instead of sh -c on Windows. The approach is correct and the change is small and focused. However, there is an inconsistency with the existing ShellCommandTool.java which uses "cmd.exe" (not "cmd") for the Windows shell, and the code structure could be slightly tightened to reduce duplication.

[AgentScopeJavaBot]

[nit] Use "cmd.exe" instead of "cmd" for consistency with the existing Windows shell invocation in ShellCommandTool.java (line 559). While "cmd" works on Windows via PATHEXT resolution, using the explicit executableable name "cmd.exe" aligns with the project's established pattern and avoids any edge-case ambiguity.

new ProcessBuilder("cmd.exe", "/c", command)

[AgentScopeJavaBot]

[nit] ProcessBuilder pb = null; followed by two branches that both assign pb is unnecessary. Consider using a ternary to select the shell command list and constructing the ProcessBuilder once, which eliminates the null initialization and avoids duplicating .directory() and .redirectErrorStream(false):

List<String> shellCmd = osName.contains("win")
        ? List.of("cmd.exe", "/c", command)
        : List.of("sh", "-c", command);
ProcessBuilder pb = new ProcessBuilder(shellCmd)
        .directory(workDir.toFile())
        .redirectErrorStream(false);

Note: ShellCommandTool.java uses the same if/else pattern, so keeping the current style is also acceptable for consistency.

[AgentScopeJavaBot]

[nit] System.getProperty("os.name") is a JVM startup property that never changes during the process lifetime. Computing it on every execute() call is a minor inefficiency. Consider extracting it to a static final field:

private static final boolean IS_WINDOWS =
        System.getProperty("os.name").toLowerCase().contains("win");

That said, ShellCommandTool.java uses the same per-call approach, so this is not blocking — just a small improvement opportunity.

[AgentScopeJavaBot]

🤖 AI Review

This PR adds Windows support to LocalFilesystemWithShell.execute() by detecting the OS and using cmd /c instead of sh -c on Windows. The approach is correct and the change is small and focused. However, there is an inconsistency with the existing ShellCommandTool.java which uses "cmd.exe" (not "cmd") for the Windows shell, and the code structure could be slightly tightened to reduce duplication.

[AgentScopeJavaBot]

[nit] Use "cmd.exe" instead of "cmd" for consistency with the existing Windows shell invocation in ShellCommandTool.java (line 559). While "cmd" works on Windows via PATHEXT resolution, using the explicit executableable name "cmd.exe" aligns with the project's established pattern and avoids any edge-case ambiguity.

new ProcessBuilder("cmd.exe", "/c", command)

[AgentScopeJavaBot]

[nit] ProcessBuilder pb = null; followed by two branches that both assign pb is unnecessary. Consider using a ternary to select the shell command list and constructing the ProcessBuilder once, which eliminates the null initialization and avoids duplicating .directory() and .redirectErrorStream(false):

List<String> shellCmd = osName.contains("win")
        ? List.of("cmd.exe", "/c", command)
        : List.of("sh", "-c", command);
ProcessBuilder pb = new ProcessBuilder(shellCmd)
        .directory(workDir.toFile())
        .redirectErrorStream(false);

Note: ShellCommandTool.java uses the same if/else pattern, so keeping the current style is also acceptable for consistency.

[AgentScopeJavaBot]

[nit] System.getProperty("os.name") is a JVM startup property that never changes during the process lifetime. Computing it on every execute() call is a minor inefficiency. Consider extracting it to a static final field:

private static final boolean IS_WINDOWS =
        System.getProperty("os.name").toLowerCase().contains("win");

That said, ShellCommandTool.java uses the same per-call approach, so this is not blocking — just a small improvement opportunity.