fix(agentctl): drop -l from task shells so PATH keeps agent CLI bin dir (#889)

This commit is contained in:
Carlos Florêncio
2026-05-13 18:39:39 +01:00
committed by GitHub
parent 7011bbcbc4
commit 08f782af22
4 changed files with 30 additions and 16 deletions
@@ -18,11 +18,14 @@ func TestDefaultShellCommand_EmptyPreferred(t *testing.T) {
if len(cmd) == 0 {
t.Fatal("expected non-empty command")
}
// On Unix, should fall back to $SHELL or /bin/sh with -l flag
// On Windows, should fall back to %COMSPEC% or powershell.exe
// On Unix the shell must be invoked WITHOUT -l so /etc/profile doesn't
// reset PATH (which would drop /data/.npm-global/bin where agent CLIs
// installed via the Settings → Agents install button live).
if runtime.GOOS != "windows" {
if cmd[len(cmd)-1] != "-l" {
t.Errorf("expected login flag -l on Unix, got %v", cmd)
for _, a := range cmd[1:] {
if a == "-l" {
t.Errorf("expected no -l flag on Unix shell, got %v", cmd)
}
}
}
}
@@ -8,8 +8,11 @@ import (
"strings"
)
// defaultShellCommand returns the command and args for starting an interactive login shell.
// On Unix, uses $SHELL (or /bin/sh) with the -l (login) flag.
// defaultShellCommand returns the command and args for starting an interactive shell.
// On Unix, uses $SHELL (or /bin/sh) with no extra flags — the shell is interactive
// via the attached PTY. We deliberately do NOT pass -l: Debian's /etc/profile
// (and /etc/profile.d/*) can overwrite PATH, dropping container-set entries like
// /data/.npm-global/bin where agent CLIs (claude, codex, ...) are installed.
func defaultShellCommand(preferredShell string) []string {
shell := resolveShellPath(preferredShell)
if shell == "" {
@@ -18,7 +21,7 @@ func defaultShellCommand(preferredShell string) []string {
if shell == "" {
shell = "/bin/sh"
}
return []string{shell, "-l"}
return []string{shell}
}
func resolveShellPath(value string) string {
@@ -95,10 +95,13 @@ func detectShell() (string, []string) {
// Unix-like systems (Linux, macOS)
// Check $SHELL but only use it if the shell actually exists
// (host's $SHELL may not be installed in Docker containers)
// (host's $SHELL may not be installed in Docker containers).
// No -l flag: Debian's /etc/profile (and /etc/profile.d/*) can overwrite
// PATH, dropping container-set entries like /data/.npm-global/bin where
// agent CLIs (claude, codex, ...) are installed.
if shell := os.Getenv("SHELL"); shell != "" {
if _, err := exec.LookPath(shell); err == nil {
return shell, []string{"-l"} // Login shell
return shell, nil
}
}
@@ -106,7 +109,7 @@ func detectShell() (string, []string) {
shells := []string{"/bin/bash", "/bin/zsh", "/bin/sh"}
for _, sh := range shells {
if _, err := os.Stat(sh); err == nil {
return sh, []string{"-l"}
return sh, nil
}
}
@@ -152,7 +155,7 @@ func defaultShellArgs(shellCommand string) []string {
}
return nil
}
return []string{"-l"}
return nil
}
// start initializes and starts the shell process
@@ -55,9 +55,12 @@ func assertUnixShell(t *testing.T, shell string, args []string) {
if _, err := os.Stat(shell); err != nil && shell != os.Getenv("SHELL") {
t.Logf("Warning: detected shell %q may not exist: %v", shell, err)
}
// Unix shells should have -l flag for login shell
if len(args) > 0 && args[0] != "-l" {
t.Errorf("expected Unix shell args to contain '-l', got %v", args)
// No -l flag: login mode lets /etc/profile reset PATH and lose the
// container-set entries needed to find agent CLIs.
for _, a := range args {
if a == "-l" {
t.Errorf("expected no -l flag in Unix shell args, got %v", args)
}
}
}
@@ -91,8 +94,10 @@ func TestDetectShellWithSHELLEnv(t *testing.T) {
if shell != "/bin/sh" {
t.Errorf("expected shell from SHELL env (/bin/sh), got %q", shell)
}
if len(args) == 0 || args[0] != "-l" {
t.Errorf("expected args to contain '-l', got %v", args)
for _, a := range args {
if a == "-l" {
t.Errorf("expected no -l flag, got %v", args)
}
}
}