From dca687987e2c376ca090dca1581e9a1979766378 Mon Sep 17 00:00:00 2001 From: Trung Nguyen Date: Fri, 28 Aug 2026 16:24:50 +0200 Subject: [PATCH 1/4] hooks/builtins: name the resolved shell in the env block MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The session_start env block previously named the OS but not the shell. When the model reads 'Operating System: Windows' it defaults to POSIX syntax by habit and emits '&&', 'grep', 'head', '2>/dev/null', 'ls -la', '$(pwd)' — none of which work under Windows PowerShell 5.1 or cmd.exe. The shell tool description already carries a shell-specific hint (shellSyntaxHint), but it sits far from the user turn and is drowned out by the agent's own system prompt; the env block sits at the top of every turn and is cached, so that is where the disambiguation belongs. Resolve the shell via the shared shellpath.DetectShell helper and print its base name plus the resolved path. Long-form dialect rules stay in the shell tool description (shellSyntaxHint) so this block stays terse and doesn't diverge from them. The base-name derivation lives in a new shellpath.ShellBaseName helper so the same logic isn't copied into hooks/builtins. No behaviour change on darwin/linux where the shell defaults to $SHELL or /bin/sh; the extra 'Shell:' line lands cleanly in the existing tab-indented env block. --- pkg/hooks/builtins/add_environment_info.go | 13 ++++++---- .../builtins/add_environment_info_test.go | 4 +++ pkg/shellpath/shellpath.go | 11 ++++++++ pkg/shellpath/shellpath_test.go | 26 +++++++++++++++++++ 4 files changed, 49 insertions(+), 5 deletions(-) diff --git a/pkg/hooks/builtins/add_environment_info.go b/pkg/hooks/builtins/add_environment_info.go index 9bf7a02ebd..1065ccbc73 100644 --- a/pkg/hooks/builtins/add_environment_info.go +++ b/pkg/hooks/builtins/add_environment_info.go @@ -6,13 +6,13 @@ import ( "runtime" "github.com/docker/docker-agent/pkg/hooks" + "github.com/docker/docker-agent/pkg/shellpath" ) // AddEnvironmentInfo is the registered name of the add_environment_info builtin. const AddEnvironmentInfo = "add_environment_info" -// addEnvironmentInfo emits cwd / git / OS / arch info as session_start -// additional context. No-op when Cwd is empty. +// addEnvironmentInfo emits cwd/git/OS/arch/shell as session_start context. func addEnvironmentInfo(_ context.Context, in *hooks.Input, _ []string) (*hooks.Output, error) { if in == nil || in.Cwd == "" { return nil, nil @@ -20,20 +20,23 @@ func addEnvironmentInfo(_ context.Context, in *hooks.Input, _ []string) (*hooks. return hooks.NewAdditionalContextOutput(hooks.EventSessionStart, environmentInfo(in.Cwd)), nil } -// environmentInfo formats the env block injected at session_start: -// working directory, git-repo status, and human-readable OS / arch. +// environmentInfo builds the block. Long-form dialect rules live in +// shellSyntaxHint (tool description) and shellDialectHint (reactive) so this +// stays terse. func environmentInfo(workingDir string) string { gitRepo := "No" if isGitRepo(workingDir) { gitRepo = "Yes" } + shellPath, _ := shellpath.DetectShell() return fmt.Sprintf(`Here is useful information about the environment you are running in: Working directory: %s Is directory a git repo: %s Operating System: %s CPU Architecture: %s - `, workingDir, gitRepo, displayOS(), displayArch()) + Shell: %s (%s) + `, workingDir, gitRepo, displayOS(), displayArch(), shellpath.ShellBaseName(shellPath), shellPath) } // displayOS returns a friendlier label for the common values of diff --git a/pkg/hooks/builtins/add_environment_info_test.go b/pkg/hooks/builtins/add_environment_info_test.go index 973713b2dc..b2c5764aad 100644 --- a/pkg/hooks/builtins/add_environment_info_test.go +++ b/pkg/hooks/builtins/add_environment_info_test.go @@ -7,6 +7,8 @@ import ( "github.com/stretchr/testify/assert" "github.com/stretchr/testify/require" + + "github.com/docker/docker-agent/pkg/shellpath" ) func TestEnvironmentInfo(t *testing.T) { @@ -49,12 +51,14 @@ func TestEnvironmentInfo(t *testing.T) { if tt.expectGit { gitStatus = "Yes" } + shellPath, _ := shellpath.DetectShell() expected := `Here is useful information about the environment you are running in: Working directory: ` + dir + ` Is directory a git repo: ` + gitStatus + ` Operating System: ` + displayOS() + ` CPU Architecture: ` + displayArch() + ` + Shell: ` + shellpath.ShellBaseName(shellPath) + ` (` + shellPath + `) ` assert.Equal(t, expected, environmentInfo(dir)) diff --git a/pkg/shellpath/shellpath.go b/pkg/shellpath/shellpath.go index 3b5fd56873..9d4914f0d4 100644 --- a/pkg/shellpath/shellpath.go +++ b/pkg/shellpath/shellpath.go @@ -7,8 +7,19 @@ import ( "os/exec" "path/filepath" "runtime" + "strings" ) +// ShellBaseName returns the lowercase shell name without extension. Splits on +// both separators so results are stable when the path came from another host OS. +func ShellBaseName(shellPath string) string { + base := shellPath + if i := strings.LastIndexAny(base, `/\`); i >= 0 { + base = base[i+1:] + } + return strings.ToLower(strings.TrimSuffix(base, filepath.Ext(base))) +} + // WindowsCmdExe returns the absolute path to cmd.exe on Windows using the // SystemRoot environment variable (e.g. C:\Windows\System32\cmd.exe). // This avoids resolving cmd.exe through PATH, which would be vulnerable diff --git a/pkg/shellpath/shellpath_test.go b/pkg/shellpath/shellpath_test.go index e66b928beb..77398380f1 100644 --- a/pkg/shellpath/shellpath_test.go +++ b/pkg/shellpath/shellpath_test.go @@ -7,6 +7,32 @@ import ( "testing" ) +func TestShellBaseName(t *testing.T) { + t.Parallel() + + tests := []struct { + path string + expected string + }{ + {path: "/bin/zsh", expected: "zsh"}, + {path: "/usr/local/bin/fish", expected: "fish"}, + {path: "/bin/sh", expected: "sh"}, + {path: `C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe`, expected: "powershell"}, + {path: `C:\Program Files\PowerShell\7\pwsh.exe`, expected: "pwsh"}, + {path: `C:\Windows\System32\cmd.exe`, expected: "cmd"}, + {path: "", expected: ""}, + } + + for _, tt := range tests { + t.Run(tt.path, func(t *testing.T) { + t.Parallel() + if got := ShellBaseName(tt.path); got != tt.expected { + t.Errorf("ShellBaseName(%q) = %q, want %q", tt.path, got, tt.expected) + } + }) + } +} + func TestWindowsCmdExe_ComSpec(t *testing.T) { t.Setenv("ComSpec", `C:\Custom\cmd.exe`) got := WindowsCmdExe() From 907fdc7ea176c660df7841d3491c180e937c4a08 Mon Sep 17 00:00:00 2001 From: Trung Nguyen Date: Fri, 28 Aug 2026 19:08:57 +0200 Subject: [PATCH 2/4] tools/shell: delegate shellBaseName to shellpath.ShellBaseName Drop the private helper and its table test that duplicated pkg/shellpath.ShellBaseName. Prevents the two copies from drifting. --- pkg/tools/builtin/shell/shell.go | 16 ++-------------- pkg/tools/builtin/shell/shell_test.go | 27 +++------------------------ 2 files changed, 5 insertions(+), 38 deletions(-) diff --git a/pkg/tools/builtin/shell/shell.go b/pkg/tools/builtin/shell/shell.go index 6c578fc8ee..532df5549a 100644 --- a/pkg/tools/builtin/shell/shell.go +++ b/pkg/tools/builtin/shell/shell.go @@ -368,7 +368,7 @@ func (t *ToolSet) Instructions() string { - Use "cwd" parameter instead of cd within commands - Combine operations with pipes, redirections, and heredocs - Non-zero exit codes return error info with output; timed-out commands are terminated`, - shellBaseName(t.handler.shell), displayOS()) + shellpath.ShellBaseName(t.handler.shell), displayOS()) } func (t *ToolSet) Tools(context.Context) ([]tools.Tool, error) { @@ -409,7 +409,7 @@ func (t *ToolSet) Stop(context.Context) error { // resolved shell is PowerShell or cmd.exe (e.g. "pwd && ls -la" is a // parse error under Windows PowerShell 5.1). func shellToolDescription(shellPath string) string { - name := shellBaseName(shellPath) + name := shellpath.ShellBaseName(shellPath) desc := fmt.Sprintf("Executes the given shell command with %s on %s.", name, displayOS()) if hint := shellSyntaxHint(name); hint != "" { desc += " " + hint @@ -435,18 +435,6 @@ func shellSyntaxHint(name string) string { } } -// shellBaseName reduces a resolved shell path to a lowercase name the -// model can recognize (C:\...\powershell.exe -> powershell, /bin/zsh -> zsh). -// Splits on both separators instead of filepath.Base so the result is -// deterministic regardless of the host OS the path came from. -func shellBaseName(shellPath string) string { - base := shellPath - if i := strings.LastIndexAny(base, `/\`); i >= 0 { - base = base[i+1:] - } - return strings.ToLower(strings.TrimSuffix(base, filepath.Ext(base))) -} - // displayOS returns a friendlier label for the common values of // runtime.GOOS, falling back to GOOS itself for anything exotic. func displayOS() string { diff --git a/pkg/tools/builtin/shell/shell_test.go b/pkg/tools/builtin/shell/shell_test.go index 00bfb2f60f..bf017c3407 100644 --- a/pkg/tools/builtin/shell/shell_test.go +++ b/pkg/tools/builtin/shell/shell_test.go @@ -15,6 +15,7 @@ import ( "github.com/stretchr/testify/require" "github.com/docker/docker-agent/pkg/config" + "github.com/docker/docker-agent/pkg/shellpath" "github.com/docker/docker-agent/pkg/tools" ) @@ -200,7 +201,7 @@ func TestShellTool_Instructions(t *testing.T) { instructions := tool.Instructions() assert.Contains(t, instructions, "Shell Tools") - assert.Contains(t, instructions, shellBaseName(tool.handler.shell), + assert.Contains(t, instructions, shellpath.ShellBaseName(tool.handler.shell), "instructions must name the resolved shell so the model uses its syntax") assert.Contains(t, instructions, displayOS()) assert.NotContains(t, instructions, "run_background_job") @@ -219,32 +220,10 @@ func TestShellTool_DescriptionNamesInterpreter(t *testing.T) { require.Len(t, allTools, 1) description := allTools[0].Description - assert.Contains(t, description, shellBaseName(tool.handler.shell)) + assert.Contains(t, description, shellpath.ShellBaseName(tool.handler.shell)) assert.Contains(t, description, displayOS()) } -func TestShellBaseName(t *testing.T) { - t.Parallel() - - tests := []struct { - path string - expected string - }{ - {path: "/bin/zsh", expected: "zsh"}, - {path: "/usr/local/bin/fish", expected: "fish"}, - {path: `C:\Windows\System32\WindowsPowerShell\v1.0\powershell.exe`, expected: "powershell"}, - {path: `C:\Program Files\PowerShell\7\pwsh.exe`, expected: "pwsh"}, - {path: `C:\Windows\System32\cmd.exe`, expected: "cmd"}, - } - - for _, tt := range tests { - t.Run(tt.path, func(t *testing.T) { - t.Parallel() - assert.Equal(t, tt.expected, shellBaseName(tt.path)) - }) - } -} - func TestResolveWorkDir(t *testing.T) { t.Parallel() From 65c2379bdc6efb7e49be880c310f4ead44b89549 Mon Sep 17 00:00:00 2001 From: Trung Nguyen Date: Fri, 28 Aug 2026 19:09:03 +0200 Subject: [PATCH 3/4] hooks/builtins: tidy addEnvironmentInfo doc comments - Drop the stale reference to shellDialectHint, which does not exist. - Restore the note that addEnvironmentInfo no-ops when Cwd is empty. - Clarify that the discarded value from shellpath.DetectShell is argsPrefix, not an error. --- pkg/hooks/builtins/add_environment_info.go | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/pkg/hooks/builtins/add_environment_info.go b/pkg/hooks/builtins/add_environment_info.go index 1065ccbc73..2eafbfd03c 100644 --- a/pkg/hooks/builtins/add_environment_info.go +++ b/pkg/hooks/builtins/add_environment_info.go @@ -13,6 +13,7 @@ import ( const AddEnvironmentInfo = "add_environment_info" // addEnvironmentInfo emits cwd/git/OS/arch/shell as session_start context. +// No-op when Cwd is empty. func addEnvironmentInfo(_ context.Context, in *hooks.Input, _ []string) (*hooks.Output, error) { if in == nil || in.Cwd == "" { return nil, nil @@ -21,14 +22,13 @@ func addEnvironmentInfo(_ context.Context, in *hooks.Input, _ []string) (*hooks. } // environmentInfo builds the block. Long-form dialect rules live in -// shellSyntaxHint (tool description) and shellDialectHint (reactive) so this -// stays terse. +// shellSyntaxHint (tool description) so this stays terse. func environmentInfo(workingDir string) string { gitRepo := "No" if isGitRepo(workingDir) { gitRepo = "Yes" } - shellPath, _ := shellpath.DetectShell() + shellPath, _ := shellpath.DetectShell() // second value is argsPrefix, unused here return fmt.Sprintf(`Here is useful information about the environment you are running in: Working directory: %s From b40e646ab59074d9a10a40cd4cd9430e166258cf Mon Sep 17 00:00:00 2001 From: Trung Nguyen Date: Mon, 31 Aug 2026 08:59:50 +0200 Subject: [PATCH 4/4] docs: mention the resolved shell in add_environment_info The two reference tables that enumerate the injected env fields were out of date after the Shell: line was added. --- docs/configuration/agents/index.md | 2 +- docs/configuration/hooks/index.md | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/docs/configuration/agents/index.md b/docs/configuration/agents/index.md index 9da240a9cb..61163733e7 100644 --- a/docs/configuration/agents/index.md +++ b/docs/configuration/agents/index.md @@ -93,7 +93,7 @@ agents: | `toolsets` | array | ✗ | List of tool configurations. See [Tool Config](../tools/index.md). | | `fallback` | object | ✗ | Automatic model failover configuration. | | `add_date` | boolean | ✗ | When `true`, injects the current date into the agent's context. | -| `add_environment_info` | boolean | ✗ | When `true`, injects working directory, OS, CPU architecture, and git info into context. | +| `add_environment_info` | boolean | ✗ | When `true`, injects working directory, OS, CPU architecture, git info, and the resolved shell into context. | | `add_prompt_files` | array | ✗ | List of file paths whose contents are appended to the system prompt. Useful for including coding standards, guidelines, or additional context. | | `add_description_parameter` | boolean | ✗ | When `true`, adds agent descriptions as a parameter in tool schemas. Helps with tool selection in multi-agent scenarios. | | `redact_secrets` | boolean | ✗ | When `true`, scrubs detected secrets (API keys, tokens, private keys, etc.) out of tool-call arguments, outgoing chat messages, and tool output before they reach a tool, the model, or downstream consumers. See [Redacting Secrets](#redacting-secrets) below. | diff --git a/docs/configuration/hooks/index.md b/docs/configuration/hooks/index.md index 34efe9d2b8..c146ab4418 100644 --- a/docs/configuration/hooks/index.md +++ b/docs/configuration/hooks/index.md @@ -206,7 +206,7 @@ Built-ins are typically zero-config and faster than equivalent shell hooks becau | Builtin | Event | Args | What it does | | ----------------------- | ----------------------------------------------------------------------------------------- | --------------------- | ---------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------------- | | `add_date` | `turn_start` | _none_ | Prepends `Today's date: YYYY-MM-DD` so the model always knows the current date. | -| `add_environment_info` | `session_start` | _none_ | Adds the working directory, git-repo status, OS, and CPU architecture. | +| `add_environment_info` | `session_start` | _none_ | Adds the working directory, git-repo status, OS, CPU architecture, and the resolved shell. | | `add_prompt_files` | `turn_start` | `[file1, file2, ...]` | Reads each named file from the workdir hierarchy (walking up) and the home directory, and appends their contents. | | `add_git_status` | `turn_start` | _none_ | Adds the output of `git status --short --branch` (no-op outside a git repo or when git isn't installed). | | `add_git_diff` | `turn_start` | _none_, or `["full"]` | Adds `git diff --stat` by default. Pass `args: ["full"]` to emit the full unified diff. Output is capped to 4 KB. |