Skip to content

Commit 56a1857

Browse files
committed
shell: don't let blank "cmd" shadow "command" alias
cmp.Or treats any non-empty string as non-zero, so a whitespace-only "cmd" would win over a valid "command" alias and trip the empty-arg guard with a misleading error. Select the alias when "cmd" is blank. Addresses PR #2481 review comments. Assisted-By: docker-agent
1 parent 6a9d771 commit 56a1857

2 files changed

Lines changed: 31 additions & 3 deletions

File tree

pkg/tools/builtin/shell.go

Lines changed: 16 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -110,7 +110,9 @@ type RunShellArgs struct {
110110
// models (particularly ones biased by Anthropic's built-in bash tool and other
111111
// ecosystems that use "command") occasionally emit "command" instead. Accepting
112112
// both prevents a wasted turn on an empty-command error while keeping the
113-
// canonical contract unchanged. When both keys are present, "cmd" wins.
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.
114116
func (a *RunShellArgs) UnmarshalJSON(data []byte) error {
115117
var raw struct {
116118
Cmd string `json:"cmd"`
@@ -121,7 +123,7 @@ func (a *RunShellArgs) UnmarshalJSON(data []byte) error {
121123
if err := json.Unmarshal(data, &raw); err != nil {
122124
return err
123125
}
124-
a.Cmd = cmp.Or(raw.Cmd, raw.Command)
126+
a.Cmd = preferNonBlank(raw.Cmd, raw.Command)
125127
a.Cwd = raw.Cwd
126128
a.Timeout = raw.Timeout
127129
return nil
@@ -143,11 +145,22 @@ func (a *RunShellBackgroundArgs) UnmarshalJSON(data []byte) error {
143145
if err := json.Unmarshal(data, &raw); err != nil {
144146
return err
145147
}
146-
a.Cmd = cmp.Or(raw.Cmd, raw.Command)
148+
a.Cmd = preferNonBlank(raw.Cmd, raw.Command)
147149
a.Cwd = raw.Cwd
148150
return nil
149151
}
150152

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+
151164
type ViewBackgroundJobArgs struct {
152165
JobID string `json:"job_id" jsonschema:"The ID of the background job to view"`
153166
}

pkg/tools/builtin/shell_test.go

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -85,6 +85,16 @@ func TestRunShellArgs_UnmarshalJSON_AcceptsCmdAndCommand(t *testing.T) {
8585
input: `{"cmd":"from-cmd","command":"from-command"}`,
8686
wantCmd: "from-cmd",
8787
},
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+
},
8898
{
8999
name: "empty object leaves cmd empty",
90100
input: `{}`,
@@ -115,6 +125,11 @@ func TestRunShellBackgroundArgs_UnmarshalJSON_AcceptsCmdAndCommand(t *testing.T)
115125
var viaCommand RunShellBackgroundArgs
116126
require.NoError(t, json.Unmarshal([]byte(`{"command":"sleep 1"}`), &viaCommand))
117127
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)
118133
}
119134

120135
// Exercises the end-to-end dispatch path: a tool-call whose raw arguments

0 commit comments

Comments
 (0)