Skip to content

Commit 6a9d771

Browse files
committed
shell: accept "command" as alias for "cmd" and improve empty-arg error
Some models (notably Claude, biased by Anthropic's built-in bash tool and other ecosystems using `command`) occasionally emit {"command": "..."} instead of the schema's canonical {"cmd": "..."}. The tool then rejected the call with "Error: empty command", wasting a turn before the model retried with the right key. This change: - Adds a custom UnmarshalJSON on RunShellArgs and RunShellBackgroundArgs that accepts both "cmd" (canonical) and "command" (alias). "cmd" wins when both are present. The advertised JSON schema is unchanged. - Rewrites the empty-command error to name the expected parameter so the model can self-correct without guessing. - Also validates params.Cmd in RunShellBackground for parity with RunShell. - Adds unit tests covering the alias path (both struct-level and end-to-end through the handler) and the actionable error message. Refs #2478
1 parent a2e9461 commit 6a9d771

2 files changed

Lines changed: 131 additions & 1 deletion

File tree

pkg/tools/builtin/shell.go

Lines changed: 46 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,51 @@ 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 both keys are present, "cmd" wins.
114+
func (a *RunShellArgs) UnmarshalJSON(data []byte) error {
115+
var raw struct {
116+
Cmd string `json:"cmd"`
117+
Command string `json:"command"`
118+
Cwd string `json:"cwd"`
119+
Timeout int `json:"timeout"`
120+
}
121+
if err := json.Unmarshal(data, &raw); err != nil {
122+
return err
123+
}
124+
a.Cmd = cmp.Or(raw.Cmd, raw.Command)
125+
a.Cwd = raw.Cwd
126+
a.Timeout = raw.Timeout
127+
return nil
128+
}
129+
105130
type RunShellBackgroundArgs struct {
106131
Cmd string `json:"cmd" jsonschema:"The shell command to execute in the background"`
107132
Cwd string `json:"cwd,omitempty" jsonschema:"The working directory to execute the command in (default: \".\")"`
108133
}
109134

135+
// UnmarshalJSON accepts both "cmd" (canonical) and "command" (common alias),
136+
// mirroring RunShellArgs.UnmarshalJSON. See its comment for rationale.
137+
func (a *RunShellBackgroundArgs) UnmarshalJSON(data []byte) error {
138+
var raw struct {
139+
Cmd string `json:"cmd"`
140+
Command string `json:"command"`
141+
Cwd string `json:"cwd"`
142+
}
143+
if err := json.Unmarshal(data, &raw); err != nil {
144+
return err
145+
}
146+
a.Cmd = cmp.Or(raw.Cmd, raw.Command)
147+
a.Cwd = raw.Cwd
148+
return nil
149+
}
150+
110151
type ViewBackgroundJobArgs struct {
111152
JobID string `json:"job_id" jsonschema:"The ID of the background job to view"`
112153
}
@@ -132,7 +173,7 @@ func statusToString(status int32) string {
132173

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

138179
timeout := h.timeout
@@ -211,6 +252,10 @@ func (h *shellHandler) runNativeCommand(timeoutCtx, ctx context.Context, command
211252
}
212253

213254
func (h *shellHandler) RunShellBackground(_ context.Context, params RunShellBackgroundArgs) (*tools.ToolCallResult, error) {
255+
if strings.TrimSpace(params.Cmd) == "" {
256+
return tools.ResultError(`Error: missing or empty "cmd" parameter. Pass the shell command as {"cmd": "..."}.`), nil
257+
}
258+
214259
counter := h.jobCounter.Add(1)
215260
jobID := fmt.Sprintf("job_%d_%d", time.Now().Unix(), counter)
216261

pkg/tools/builtin/shell_test.go

Lines changed: 85 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,90 @@ 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: "empty object leaves cmd empty",
90+
input: `{}`,
91+
wantCmd: "",
92+
},
93+
}
94+
95+
for _, tt := range tests {
96+
t.Run(tt.name, func(t *testing.T) {
97+
t.Parallel()
98+
var got RunShellArgs
99+
require.NoError(t, json.Unmarshal([]byte(tt.input), &got))
100+
assert.Equal(t, tt.wantCmd, got.Cmd)
101+
assert.Equal(t, tt.wantCwd, got.Cwd)
102+
assert.Equal(t, tt.wantTO, got.Timeout)
103+
})
104+
}
105+
}
106+
107+
func TestRunShellBackgroundArgs_UnmarshalJSON_AcceptsCmdAndCommand(t *testing.T) {
108+
t.Parallel()
109+
110+
var viaCmd RunShellBackgroundArgs
111+
require.NoError(t, json.Unmarshal([]byte(`{"cmd":"sleep 1","cwd":"/tmp"}`), &viaCmd))
112+
assert.Equal(t, "sleep 1", viaCmd.Cmd)
113+
assert.Equal(t, "/tmp", viaCmd.Cwd)
114+
115+
var viaCommand RunShellBackgroundArgs
116+
require.NoError(t, json.Unmarshal([]byte(`{"command":"sleep 1"}`), &viaCommand))
117+
assert.Equal(t, "sleep 1", viaCommand.Cmd)
118+
}
119+
120+
// Exercises the end-to-end dispatch path: a tool-call whose raw arguments
121+
// use "command" instead of "cmd" must execute normally rather than return
122+
// the missing-parameter error.
123+
func TestShellTool_HandlerAcceptsCommandAlias(t *testing.T) {
124+
tool := NewShellTool(nil, &config.RuntimeConfig{Config: config.Config{WorkingDir: t.TempDir()}})
125+
126+
var params RunShellArgs
127+
require.NoError(t, json.Unmarshal([]byte(`{"command":"echo hello-from-alias"}`), &params))
128+
129+
result, err := tool.handler.RunShell(t.Context(), params)
130+
require.NoError(t, err)
131+
assert.Contains(t, result.Output, "hello-from-alias")
132+
}
133+
134+
func TestShellTool_HandlerMissingCmdReturnsActionableError(t *testing.T) {
135+
tool := NewShellTool(nil, &config.RuntimeConfig{Config: config.Config{WorkingDir: t.TempDir()}})
136+
137+
result, err := tool.handler.RunShell(t.Context(), RunShellArgs{})
138+
require.NoError(t, err)
139+
assert.Contains(t, result.Output, `"cmd"`,
140+
"error must name the expected parameter so the model can self-correct")
141+
}
142+
58143
func TestShellTool_HandlerError(t *testing.T) {
59144
tool := NewShellTool(nil, &config.RuntimeConfig{Config: config.Config{WorkingDir: t.TempDir()}})
60145

0 commit comments

Comments
 (0)