Skip to content

Commit 8070817

Browse files
llama-atmosphere-agent: make ShellToolTest run on Windows too
ShellToolTest used POSIX commands (ls, sleep, printf, `; exit 3`) and failed 4 of 5 tests on Windows, where ShellTool runs cmd.exe /c. Each test now picks its command line with ShellTool.isWindows() -- the same detection ShellTool.run uses to choose the shell -- so it always speaks the shell the tool actually starts: - ls / dir /b - `echo boom 1>&2; exit 3` / `echo boom 1>&2 & exit 3` - truncation: echo on both, the expected length counts the platform's line separator (\n from sh, \r\n from cmd.exe) - sleep 30 / ping -n 30 127.0.0.1 >nul (also a child of the shell, so the descendant kill stays covered) Verified: Windows 11 (all 31 agent tests), Linux in maven:3.9-eclipse-temurin-21 (ShellToolTest 5/5, AgentOptionsTest 11/11). Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01E2h8gXyyE5UeimkL9vQv9G
1 parent 96cbbc7 commit 8070817

2 files changed

Lines changed: 37 additions & 10 deletions

File tree

‎CLAUDE.md‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2214,9 +2214,11 @@ tools are only for files" — with the tool registered and `docker` on `PATH`; a
22142214
refused too, so it was the prompt, not the chat history. Without `--allow-shell` the prompt says commands
22152215
are unavailable and names the flag, so the model does not invent its own limitation. Pinned by
22162216
`AgentOptionsTest.defaultSystemPromptIsGeneralPurposeAndAllowsAnyCommandWithTheShell` and
2217-
`shellToolDescriptionDoesNotNarrowItToTheProject`. `ShellToolTest` uses POSIX commands (`ls`, `sleep`,
2218-
`exit 3`), so it passes only on Linux/macOS — CI runs it on Linux; on Windows run the suite with
2219-
`-Dtest=!ShellToolTest`.
2217+
`shellToolDescriptionDoesNotNarrowItToTheProject`. `ShellToolTest` runs on every platform: each test
2218+
picks its command line with `ShellTool.isWindows()` — the same detection `ShellTool.run` uses to choose
2219+
`cmd.exe /c` over `sh -c` — so `ls`/`dir /b`, `sleep 30`/`ping -n 30 127.0.0.1 >nul`, and the truncation
2220+
test counts the platform's line separator. It used plain POSIX commands before and failed 4 of 5 on
2221+
Windows; never skip it per OS, give a new test both command forms instead.
22202222

22212223
**Version bump note.** The pom's `llama.version` property is the **release** version, not the
22222224
reactor's `-SNAPSHOT` (CI always overrides it, so a not-yet-published default never breaks CI).

‎llama-atmosphere-agent/src/test/java/net/ladenthin/llama/atmosphere/ShellToolTest.java‎

Lines changed: 32 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -17,17 +17,33 @@
1717
import org.junit.jupiter.api.Test;
1818
import org.junit.jupiter.api.io.TempDir;
1919

20+
/**
21+
* Runs on every platform: each test picks its command line with {@link ShellTool#isWindows()}, the same
22+
* detection {@link ShellTool#run} uses to choose between {@code cmd.exe /c} and {@code sh -c}, so the test
23+
* always speaks the shell the tool actually starts.
24+
*/
2025
class ShellToolTest {
2126

2227
@TempDir
2328
Path workspace;
2429

30+
/**
31+
* The command line for the shell {@link ShellTool} starts on this platform.
32+
*
33+
* @param posix the {@code sh} form
34+
* @param windows the {@code cmd.exe} form
35+
* @return the form matching {@link ShellTool#isWindows()}
36+
*/
37+
private static String shell(String posix, String windows) {
38+
return ShellTool.isWindows() ? windows : posix;
39+
}
40+
2541
@Test
2642
void runsInTheWorkspaceAndReportsExitCodeAndOutput() throws Exception {
2743
Files.writeString(workspace.resolve("marker.txt"), "x");
2844
ToolDefinition tool = ShellTool.definition(workspace, Duration.ofSeconds(30), 10_000);
2945

30-
Object result = tool.executor().execute(Map.of("command", "ls"));
46+
Object result = tool.executor().execute(Map.of("command", shell("ls", "dir /b")));
3147

3248
assertThat(tool.name(), is(ShellTool.TOOL_NAME));
3349
assertThat(String.valueOf(result), startsWith("exit code: 0"));
@@ -38,7 +54,8 @@ void runsInTheWorkspaceAndReportsExitCodeAndOutput() throws Exception {
3854
void nonZeroExitAndStderrAreReturnedNotThrown() throws Exception {
3955
ToolDefinition tool = ShellTool.definition(workspace, Duration.ofSeconds(30), 10_000);
4056

41-
Object result = tool.executor().execute(Map.of("command", "echo boom 1>&2; exit 3"));
57+
Object result =
58+
tool.executor().execute(Map.of("command", shell("echo boom 1>&2; exit 3", "echo boom 1>&2 & exit 3")));
4259

4360
assertThat(String.valueOf(result), startsWith("exit code: 3"));
4461
assertThat(String.valueOf(result), containsString("boom"));
@@ -53,15 +70,23 @@ void missingCommandIsAnErrorString() throws Exception {
5370

5471
@Test
5572
void outputIsTruncatedToTheTail() throws Exception {
56-
String result = ShellTool.run(workspace, "printf 'aaaaaaaaaaaaaaaaaaaaZZ'", Duration.ofSeconds(30), 5);
57-
58-
assertThat(result, containsString("[output truncated to the last 5 of 22 characters]"));
59-
assertThat(result, containsString("aaaZZ"));
73+
// echo is the one output command both shells share; it ends the line with the platform's
74+
// separator (\n from sh, \r\n from cmd.exe), which is part of the counted output.
75+
String newline = ShellTool.isWindows() ? "\r\n" : "\n";
76+
String result = ShellTool.run(workspace, "echo aaaaaaaaaaaaaaaaaaaaZZ", Duration.ofSeconds(30), 5);
77+
78+
assertThat(
79+
result,
80+
containsString("[output truncated to the last 5 of " + (22 + newline.length()) + " characters]"));
81+
assertThat(result, containsString("ZZ" + newline));
6082
}
6183

6284
@Test
6385
void timeoutKillsTheProcess() throws Exception {
64-
String result = ShellTool.run(workspace, "sleep 30", Duration.ofMillis(300), 10_000);
86+
// cmd.exe has no sleep; ping waits about one second per echo request. Like `sh -c "sleep 30"`,
87+
// it is a child of the shell, so this also covers killing the descendants.
88+
String result = ShellTool.run(
89+
workspace, shell("sleep 30", "ping -n 30 127.0.0.1 >nul"), Duration.ofMillis(300), 10_000);
6590

6691
assertThat(result, startsWith("exit code: (killed after 0 s)"));
6792
}

0 commit comments

Comments
 (0)