diff --git a/internal/proc/path_other.go b/internal/proc/path_other.go index c02e98be..d00799bd 100644 --- a/internal/proc/path_other.go +++ b/internal/proc/path_other.go @@ -166,25 +166,51 @@ func shellEnv() (string, map[string]string) { } ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second) defer cancel() - // interactive too, since many put PATH in .zshrc/.bashrc; the marker - // tells the answer apart from whatever the profile prints, and a NUL - // (which no variable can hold) parts the values: PATH first, then each - // of agentenv.Vars in order - const mark = "__magpie_path__" - format, args := mark+"%s", ` "$PATH"` + // interactive too, since many put PATH in .zshrc/.bashrc + cmd := CommandContext(ctx, sh, "-ilc", shellProbe(sh)) + cmd.Stdin = nil + out, _ := cmd.Output() + return parseShellEnv(string(out), shellMark) +} + +// shellMark tells shellEnv's answer apart from whatever the profile prints. +const shellMark = "__magpie_path__" + +// shellProbe is the command the login shell sh runs to print its PATH and +// agentenv.Vars, between two marks. A NUL (which no variable can hold) +// parts the values: PATH first, then each of agentenv.Vars in order. +// +// A POSIX shell (zsh, bash, fish too) expands "$PATH" itself. nushell +// doesn't: a double-quoted string is literal there, so it printed "$PATH", +// "$CLAUDE_CONFIG_DIR", ... and magpie set each variable to its own name +// (#738: then Claude sign-in failed with "mkdir $CLAUDE_CONFIG_DIR: +// read-only file system", and the account lapsed minutes later, Claude +// Code's sign-in looked for under that folder). nushell has no POSIX +// quoting to ask with, and its PATH is a list, so it is asked to run +// /bin/sh with the same printf: nushell hands it the environment as a +// child sees it, PATH joined back into one string, and sh expands the +// references. The outer command is in nushell's single quotes, where a +// backslash is a backslash, and the printf format in sh's double quotes, +// where \0 reaches printf as it is. +func shellProbe(sh string) string { + format, args := shellMark+"%s", ` "$PATH"` for _, v := range agentenv.Vars { format += `\0%s` args += ` "$` + v + `"` } - cmd := CommandContext(ctx, sh, "-ilc", "printf '"+format+mark+"'"+args) - cmd.Stdin = nil - out, _ := cmd.Output() - return parseShellEnv(string(out), mark) + if filepath.Base(sh) == "nu" { + return `^/bin/sh -c 'printf "` + format + shellMark + `"` + args + `'` + } + return "printf '" + format + shellMark + "'" + args } // parseShellEnv reads shellEnv's answer out of what the shell printed. The // variables are nil when their count isn't agentenv.Vars' (a printf that -// doesn't know \0); PATH alone is still taken then. +// doesn't know \0); PATH alone is still taken then. A value that is the +// variable's own reference ("$CODEX_HOME") is a shell that printed the +// text instead of expanding it (nushell before shellProbe knew it; a shell +// with other quoting still), not a folder: it counts as unset, as does a +// PATH of "$PATH". func parseShellEnv(s, mark string) (string, map[string]string) { i := strings.Index(s, mark) if i < 0 { @@ -196,6 +222,9 @@ func parseShellEnv(s, mark string) (string, map[string]string) { return "", nil } f := strings.Split(s[:j], "\x00") + if f[0] == "$PATH" { + f[0] = "" + } if len(f) != len(agentenv.Vars)+1 { if len(f) == 1 { return f[0], nil @@ -204,6 +233,9 @@ func parseShellEnv(s, mark string) (string, map[string]string) { } vars := make(map[string]string, len(agentenv.Vars)) for k, v := range agentenv.Vars { + if f[k+1] == "$"+v { + f[k+1] = "" + } vars[v] = f[k+1] } return f[0], vars diff --git a/internal/proc/path_other_test.go b/internal/proc/path_other_test.go index f72b84ea..7ad0952a 100644 --- a/internal/proc/path_other_test.go +++ b/internal/proc/path_other_test.go @@ -9,6 +9,8 @@ import ( "strings" "testing" "time" + + "github.com/yetone/magpie/internal/agentenv" ) // A desktop app with launchd's PATH finds a claude installed under a custom @@ -82,3 +84,60 @@ func TestUserPathTakesAgentVars(t *testing.T) { t.Fatalf("PATH lost: %q", os.Getenv("PATH")) } } + +// A login shell that doesn't expand "$CLAUDE_CONFIG_DIR" prints the +// reference itself. That is no folder: the variable counts as unset, +// rather than magpie setting CLAUDE_CONFIG_DIR to "$CLAUDE_CONFIG_DIR" and +// then looking for Claude Code's sign-in under it (#738). +func TestParseShellEnvUnexpanded(t *testing.T) { + s := shellMark + "$PATH" + for _, v := range agentenv.Vars { + s += "\x00$" + v + } + p, vars := parseShellEnv("hello from the profile\n"+s+shellMark, shellMark) + if p != "" { + t.Fatalf("PATH = %q, want none", p) + } + if len(vars) != len(agentenv.Vars) { + t.Fatalf("%d variables, want %d", len(vars), len(agentenv.Vars)) + } + for v, got := range vars { + if got != "" { + t.Fatalf("%s = %q, want none", v, got) + } + } +} + +// nushell is asked in its own way, and answers with its PATH and the +// variables a child of it gets, as a POSIX shell does; where nushell isn't +// installed, the command's shape is all that can be checked. +func TestShellEnvNushell(t *testing.T) { + probe := shellProbe("/opt/homebrew/bin/nu") + if !strings.HasPrefix(probe, "^/bin/sh -c 'printf \""+shellMark+"%s") || !strings.HasSuffix(probe, `"$`+agentenv.Vars[len(agentenv.Vars)-1]+`"'`) { + t.Fatalf("nushell's probe: %q", probe) + } + if strings.Contains(probe, `'`+shellMark) || strings.Count(probe, "'") != 2 { + t.Fatalf("nushell's probe must hold sh's command in one pair of single quotes: %q", probe) + } + nu, err := exec.LookPath("nu") + if err != nil { + t.Skip("nushell isn't installed") + } + t.Setenv("CODEX_HOME", "/from/the/environment") + t.Setenv("CLAUDE_CONFIG_DIR", "") + os.Unsetenv("CLAUDE_CONFIG_DIR") + out, err := exec.Command(nu, "-ilc", shellProbe(nu)).Output() + if err != nil { + t.Fatalf("nu: %v", err) + } + p, vars := parseShellEnv(string(out), shellMark) + if p == "" || strings.Contains(p, "$PATH") { + t.Fatalf("PATH from nushell: %q", p) + } + if got := vars["CODEX_HOME"]; got != "/from/the/environment" { + t.Fatalf("CODEX_HOME = %q: nushell's environment didn't reach sh", got) + } + if got := vars["CLAUDE_CONFIG_DIR"]; got != "" { + t.Fatalf("CLAUDE_CONFIG_DIR = %q, want none", got) + } +}