Skip to content

Commit 87fbdb0

Browse files
authored
🤖 Fix niceness bash escaping regression (#167)
Fixes the regression introduced in PR #162 where complex multi-line bash scripts with conditionals would fail with exit code 2. ## Problem The original implementation wrapped commands using `JSON.stringify`, which converted newlines to literal `\n` strings: ```bash nice -n 19 bash -c "if [ \0 -ne 0 ]; then\n echo \"test\"\nfi" ``` This broke bash syntax, causing scripts with conditionals and multi-line logic to fail. ## Solution Spawn `nice` directly with proper arguments array instead of string wrapping: ```typescript spawn("nice", ["-n", "19", "bash", "-c", script], ...) ``` This avoids all escaping issues while maintaining the same niceness functionality. ## Testing Added comprehensive test coverage: - ✅ Complex multi-line scripts with conditionals (reproduces the git status regression) - ✅ Exit code handling with niceness - ✅ Simple commands with niceness All 36 tests pass including the new niceness test suite. _Generated with `cmux`_
1 parent 16c79e1 commit 87fbdb0

File tree

2 files changed

+91
-7
lines changed

2 files changed

+91
-7
lines changed

src/services/tools/bash.test.ts

Lines changed: 85 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -677,3 +677,88 @@ describe("bash tool", () => {
677677
}
678678
});
679679
});
680+
681+
describe("niceness parameter", () => {
682+
it("should execute complex multi-line scripts with niceness", async () => {
683+
const tool = createBashTool({ cwd: process.cwd(), niceness: 19 });
684+
685+
// Complex script with conditionals, similar to GIT_STATUS_SCRIPT
686+
const args: BashToolArgs = {
687+
script: `
688+
# Test complex script with conditionals
689+
VALUE=$(echo "test")
690+
691+
if [ -z "$VALUE" ]; then
692+
echo "ERROR: Value is empty"
693+
exit 1
694+
fi
695+
696+
# Another conditional check
697+
RESULT=$(echo "success")
698+
if [ $? -ne 0 ]; then
699+
echo "ERROR: Command failed"
700+
exit 2
701+
fi
702+
703+
echo "---OUTPUT---"
704+
echo "$VALUE"
705+
echo "$RESULT"
706+
`,
707+
timeout_secs: 5,
708+
max_lines: 100,
709+
};
710+
711+
const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult;
712+
713+
expect(result.success).toBe(true);
714+
if (result.success) {
715+
expect(result.output).toContain("---OUTPUT---");
716+
expect(result.output).toContain("test");
717+
expect(result.output).toContain("success");
718+
expect(result.exitCode).toBe(0);
719+
}
720+
});
721+
722+
it("should handle exit codes correctly with niceness", async () => {
723+
const tool = createBashTool({ cwd: process.cwd(), niceness: 19 });
724+
725+
// Script that should exit with code 2
726+
const args: BashToolArgs = {
727+
script: `
728+
RESULT=$(false)
729+
if [ $? -ne 0 ]; then
730+
echo "Command failed as expected"
731+
exit 2
732+
fi
733+
`,
734+
timeout_secs: 5,
735+
max_lines: 100,
736+
};
737+
738+
const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult;
739+
740+
expect(result.success).toBe(false);
741+
expect(result.exitCode).toBe(2);
742+
// Error message includes stderr output
743+
if (!result.success) {
744+
expect(result.error).toMatch(/Command failed as expected|Command exited with code 2/);
745+
}
746+
});
747+
748+
it("should execute simple commands with niceness", async () => {
749+
const tool = createBashTool({ cwd: process.cwd(), niceness: 10 });
750+
const args: BashToolArgs = {
751+
script: "echo hello",
752+
timeout_secs: 5,
753+
max_lines: 100,
754+
};
755+
756+
const result = (await tool.execute!(args, mockToolCallOptions)) as BashToolResult;
757+
758+
expect(result.success).toBe(true);
759+
if (result.success) {
760+
expect(result.output).toBe("hello");
761+
expect(result.exitCode).toBe(0);
762+
}
763+
});
764+
});

src/services/tools/bash.ts

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -94,16 +94,15 @@ export const createBashTool: ToolFactory = (config: ToolConfiguration) => {
9494
}
9595

9696
// Create the process with `using` for automatic cleanup
97-
// If niceness is specified, wrap the command with nice
98-
const finalScript =
97+
// If niceness is specified, spawn nice directly to avoid escaping issues
98+
const spawnCommand = config.niceness !== undefined ? "nice" : "bash";
99+
const spawnArgs =
99100
config.niceness !== undefined
100-
? `nice -n ${config.niceness} bash -c ${JSON.stringify(script)}`
101-
: script;
102-
const finalCommand =
103-
config.niceness !== undefined ? ["bash", "-c", finalScript] : ["bash", "-c", script];
101+
? ["-n", config.niceness.toString(), "bash", "-c", script]
102+
: ["-c", script];
104103

105104
using childProcess = new DisposableProcess(
106-
spawn(finalCommand[0], finalCommand.slice(1), {
105+
spawn(spawnCommand, spawnArgs, {
107106
cwd: config.cwd,
108107
env: {
109108
...process.env,

0 commit comments

Comments
 (0)