Skip to content

Commit d8f1bfa

Browse files
dgageotdgageot
authored
Merge pull request #2481 from trungutt/fix/shell-accept-command-alias
shell: accept `command` as alias for `cmd` and improve empty-arg error
2 parents db0dfdb + 56a1857 commit d8f1bfa

2 files changed

Lines changed: 159 additions & 1 deletion

File tree

‎pkg/tools/builtin/shell.go‎

Lines changed: 59 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@ import (
44
"bytes"
55
"cmp"
66
"context"
7+
"encoding/json"
78
"errors"
89
"fmt"
910
"log/slog"
@@ -102,11 +103,64 @@ type RunShellArgs struct {
102103
Timeout int `json:"timeout,omitempty" jsonschema:"Command execution timeout in seconds (default: 30)"`
103104
}
104105

106+
// UnmarshalJSON accepts both the canonical "cmd" key and the common alias
107+
// "command" for the shell command parameter.
108+
//
109+
// The advertised schema still declares "cmd" as the canonical name, but many
110+
// models (particularly ones biased by Anthropic's built-in bash tool and other
111+
// ecosystems that use "command") occasionally emit "command" instead. Accepting
112+
// both prevents a wasted turn on an empty-command error while keeping the
113+
// canonical contract unchanged. When "cmd" is present with a non-blank value
114+
// it wins; a blank (empty or whitespace-only) "cmd" falls back to "command"
115+
// so a valid alias is not silently shadowed.
116+
func (a *RunShellArgs) UnmarshalJSON(data []byte) error {
117+
var raw struct {
118+
Cmd string `json:"cmd"`
119+
Command string `json:"command"`
120+
Cwd string `json:"cwd"`
121+
Timeout int `json:"timeout"`
122+
}
123+
if err := json.Unmarshal(data, &raw); err != nil {
124+
return err
125+
}
126+
a.Cmd = preferNonBlank(raw.Cmd, raw.Command)
127+
a.Cwd = raw.Cwd
128+
a.Timeout = raw.Timeout
129+
return nil
130+
}
131+
105132
type RunShellBackgroundArgs struct {
106133
Cmd string `json:"cmd" jsonschema:"The shell command to execute in the background"`
107134
Cwd string `json:"cwd,omitempty" jsonschema:"The working directory to execute the command in (default: \".\")"`
108135
}
109136

137+
// UnmarshalJSON accepts both "cmd" (canonical) and "command" (common alias),
138+
// mirroring RunShellArgs.UnmarshalJSON. See its comment for rationale.
139+
func (a *RunShellBackgroundArgs) UnmarshalJSON(data []byte) error {
140+
var raw struct {
141+
Cmd string `json:"cmd"`
142+
Command string `json:"command"`
143+
Cwd string `json:"cwd"`
144+
}
145+
if err := json.Unmarshal(data, &raw); err != nil {
146+
return err
147+
}
148+
a.Cmd = preferNonBlank(raw.Cmd, raw.Command)
149+
a.Cwd = raw.Cwd
150+
return nil
151+
}
152+
153+
// preferNonBlank returns primary when it has a non-whitespace character;
154+
// otherwise it returns fallback. The chosen value is returned unmodified so
155+
// that whitespace inside a legitimate command (e.g. trailing newlines in a
156+
// heredoc) is preserved.
157+
func preferNonBlank(primary, fallback string) string {
158+
if strings.TrimSpace(primary) != "" {
159+
return primary
160+
}
161+
return fallback
162+
}
163+
110164
type ViewBackgroundJobArgs struct {
111165
JobID string `json:"job_id" jsonschema:"The ID of the background job to view"`
112166
}
@@ -132,7 +186,7 @@ func statusToString(status int32) string {
132186

133187
func (h *shellHandler) RunShell(ctx context.Context, params RunShellArgs) (*tools.ToolCallResult, error) {
134188
if strings.TrimSpace(params.Cmd) == "" {
135-
return tools.ResultError("Error: empty command"), nil
189+
return tools.ResultError(`Error: missing or empty "cmd" parameter. Pass the shell command as {"cmd": "..."}.`), nil
136190
}
137191

138192
timeout := h.timeout
@@ -214,6 +268,10 @@ func (h *shellHandler) runNativeCommand(timeoutCtx, ctx context.Context, command
214268
}
215269

216270
func (h *shellHandler) RunShellBackground(_ context.Context, params RunShellBackgroundArgs) (*tools.ToolCallResult, error) {
271+
if strings.TrimSpace(params.Cmd) == "" {
272+
return tools.ResultError(`Error: missing or empty "cmd" parameter. Pass the shell command as {"cmd": "..."}.`), nil
273+
}
274+
217275
counter := h.jobCounter.Add(1)
218276
jobID := fmt.Sprintf("job_%d_%d", time.Now().Unix(), counter)
219277

‎pkg/tools/builtin/shell_test.go‎

Lines changed: 100 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
11
package builtin
22

33
import (
4+
"encoding/json"
45
"os"
56
"os/exec"
67
"runtime"
@@ -55,6 +56,105 @@ func TestShellTool_HandlerWithCwd(t *testing.T) {
5556
assert.Contains(t, result.Output, tmpDir)
5657
}
5758

59+
func TestRunShellArgs_UnmarshalJSON_AcceptsCmdAndCommand(t *testing.T) {
60+
t.Parallel()
61+
62+
tests := []struct {
63+
name string
64+
input string
65+
wantCmd string
66+
wantCwd string
67+
wantTO int
68+
}{
69+
{
70+
name: "canonical cmd",
71+
input: `{"cmd":"ls -la","cwd":"/tmp","timeout":10}`,
72+
wantCmd: "ls -la",
73+
wantCwd: "/tmp",
74+
wantTO: 10,
75+
},
76+
{
77+
name: "alias command",
78+
input: `{"command":"ls -la","cwd":"/tmp","timeout":10}`,
79+
wantCmd: "ls -la",
80+
wantCwd: "/tmp",
81+
wantTO: 10,
82+
},
83+
{
84+
name: "both present cmd wins",
85+
input: `{"cmd":"from-cmd","command":"from-command"}`,
86+
wantCmd: "from-cmd",
87+
},
88+
{
89+
name: "blank cmd falls back to command alias",
90+
input: `{"cmd":" ","command":"from-command"}`,
91+
wantCmd: "from-command",
92+
},
93+
{
94+
name: "empty cmd falls back to command alias",
95+
input: `{"cmd":"","command":"from-command"}`,
96+
wantCmd: "from-command",
97+
},
98+
{
99+
name: "empty object leaves cmd empty",
100+
input: `{}`,
101+
wantCmd: "",
102+
},
103+
}
104+
105+
for _, tt := range tests {
106+
t.Run(tt.name, func(t *testing.T) {
107+
t.Parallel()
108+
var got RunShellArgs
109+
require.NoError(t, json.Unmarshal([]byte(tt.input), &got))
110+
assert.Equal(t, tt.wantCmd, got.Cmd)
111+
assert.Equal(t, tt.wantCwd, got.Cwd)
112+
assert.Equal(t, tt.wantTO, got.Timeout)
113+
})
114+
}
115+
}
116+
117+
func TestRunShellBackgroundArgs_UnmarshalJSON_AcceptsCmdAndCommand(t *testing.T) {
118+
t.Parallel()
119+
120+
var viaCmd RunShellBackgroundArgs
121+
require.NoError(t, json.Unmarshal([]byte(`{"cmd":"sleep 1","cwd":"/tmp"}`), &viaCmd))
122+
assert.Equal(t, "sleep 1", viaCmd.Cmd)
123+
assert.Equal(t, "/tmp", viaCmd.Cwd)
124+
125+
var viaCommand RunShellBackgroundArgs
126+
require.NoError(t, json.Unmarshal([]byte(`{"command":"sleep 1"}`), &viaCommand))
127+
assert.Equal(t, "sleep 1", viaCommand.Cmd)
128+
129+
// A blank "cmd" must not shadow a valid "command" alias.
130+
var blankCmd RunShellBackgroundArgs
131+
require.NoError(t, json.Unmarshal([]byte(`{"cmd":" ","command":"sleep 1"}`), &blankCmd))
132+
assert.Equal(t, "sleep 1", blankCmd.Cmd)
133+
}
134+
135+
// Exercises the end-to-end dispatch path: a tool-call whose raw arguments
136+
// use "command" instead of "cmd" must execute normally rather than return
137+
// the missing-parameter error.
138+
func TestShellTool_HandlerAcceptsCommandAlias(t *testing.T) {
139+
tool := NewShellTool(nil, &config.RuntimeConfig{Config: config.Config{WorkingDir: t.TempDir()}})
140+
141+
var params RunShellArgs
142+
require.NoError(t, json.Unmarshal([]byte(`{"command":"echo hello-from-alias"}`), &params))
143+
144+
result, err := tool.handler.RunShell(t.Context(), params)
145+
require.NoError(t, err)
146+
assert.Contains(t, result.Output, "hello-from-alias")
147+
}
148+
149+
func TestShellTool_HandlerMissingCmdReturnsActionableError(t *testing.T) {
150+
tool := NewShellTool(nil, &config.RuntimeConfig{Config: config.Config{WorkingDir: t.TempDir()}})
151+
152+
result, err := tool.handler.RunShell(t.Context(), RunShellArgs{})
153+
require.NoError(t, err)
154+
assert.Contains(t, result.Output, `"cmd"`,
155+
"error must name the expected parameter so the model can self-correct")
156+
}
157+
58158
func TestShellTool_HandlerError(t *testing.T) {
59159
tool := NewShellTool(nil, &config.RuntimeConfig{Config: config.Config{WorkingDir: t.TempDir()}})
60160

0 commit comments

Comments
 (0)