MUL-3944: Fix daemon agent discovery around hook wrappers (#4817)

* Fix agent path discovery around hook wrappers Co-authored-by: multica-agent <github@multica.ai> * Fix login shell hook wrapper discovery Co-authored-by: multica-agent <github@multica.ai> --------- Co-authored-by: multica-agent <github@multica.ai>
2026-07-05 13:29:44 +02:00 · 2026-07-03 12:35:03 +08:00
parent 8797667fac
commit d4f57aff7a
3 changed files with 231 additions and 5 deletions
--- a/CLI_AND_DAEMON.md
+++ b/CLI_AND_DAEMON.md
@@ -227,6 +227,8 @@ Agent-specific overrides:
 | `MULTICA_TRAECLI_PATH` | Custom path to the `traecli` binary |
 | `MULTICA_TRAECLI_MODEL` | Override the Trae model used (a model id from your logged-in traecli catalog, e.g. `Doubao-Seed-2.1-Pro`) |

+If a previously generated `~/.multica/hooks` wrapper is first on `PATH` and calls the same command name again, the daemon skips that hooks directory during built-in agent discovery and records the real binary path behind it. If your interactive shell still recurses when you run `claude`, `codex`, or `hermes` manually, remove the hooks entry from your shell startup file or replace the wrapper body with an absolute `exec /path/to/real-binary "$@"`.
+
 The daemon launches Qoder as `qodercli --yolo --acp`, matching Qoder’s ACP “bypass permissions” mode so tool runs do not block on interactive approval in headless runs.

 `MULTICA_CLAUDE_ARGS` and `MULTICA_CODEX_ARGS` are parsed with POSIX shellword quoting, so values such as `--model "gpt-5.1 codex" --sandbox read-only` are split like a shell command line. Agent arguments are applied in this order: hardcoded Multica defaults, daemon-wide env defaults, then per-agent `custom_args` from the task.
--- a/server/internal/daemon/config.go
+++ b/server/internal/daemon/config.go
@@ -222,9 +222,9 @@ func LoadConfig(overrides Overrides) (Config, error) {
 	}
 	probe := func(envVar, defaultCmd, modelEnv string) (AgentEntry, bool) {
 		cmd := envOrDefault(envVar, defaultCmd)
-		if _, err := exec.LookPath(cmd); err == nil {
+		if path, err := resolveAgentExecutablePath(cmd); err == nil {
 			return AgentEntry{
-				Path:  cmd,
+				Path:  path,
 				Model: strings.TrimSpace(os.Getenv(modelEnv)),
 			}, true
 		}
@@ -298,9 +298,9 @@ func LoadConfig(overrides Overrides) (Config, error) {
 		agents["antigravity"] = e
 	}
 	qoderPath := envOrDefault("MULTICA_QODER_PATH", "qodercli")
-	if _, err := exec.LookPath(qoderPath); err == nil {
+	if path, err := resolveAgentExecutablePath(qoderPath); err == nil {
 		agents["qoder"] = AgentEntry{
-			Path:  qoderPath,
+			Path:  path,
 			Model: strings.TrimSpace(os.Getenv("MULTICA_QODER_MODEL")),
 		}
 	}
@@ -654,6 +654,98 @@ func shellArgsFromEnv(name string) ([]string, error) {
 	return args, nil
 }

+// resolveAgentExecutablePath returns the concrete executable path the daemon
+// should keep for an agent command. Bare command names are pinned to the path
+// resolved during startup so later PATH changes cannot redirect task launches.
+// When ~/.multica/hooks shadows a real agent binary, skip that hooks directory:
+// previously generated hook wrappers can execute the same command name and
+// recurse forever if the daemon records or launches the wrapper.
+func resolveAgentExecutablePath(cmd string) (string, error) {
+	resolved, err := exec.LookPath(cmd)
+	if err != nil {
+		return "", err
+	}
+	if strings.ContainsAny(cmd, "/\\") {
+		return resolved, nil
+	}
+	if isInMulticaHooksDir(resolved) {
+		if unshadowed, err := lookPathExcludingMulticaHooks(cmd); err == nil {
+			return unshadowed, nil
+		}
+	}
+	return canonicalExecutablePath(resolved), nil
+}
+
+func lookPathExcludingMulticaHooks(cmd string) (string, error) {
+	for _, dir := range filepath.SplitList(os.Getenv("PATH")) {
+		if dir == "" {
+			dir = "."
+		}
+		if isMulticaHooksDir(dir) {
+			continue
+		}
+		candidate := filepath.Join(dir, cmd)
+		if isExecutableFile(candidate) {
+			return canonicalExecutablePath(candidate), nil
+		}
+	}
+	return "", exec.ErrNotFound
+}
+
+func isInMulticaHooksDir(path string) bool {
+	if path == "" {
+		return false
+	}
+	return isMulticaHooksDir(filepath.Dir(path))
+}
+
+func isMulticaHooksDir(dir string) bool {
+	home, err := os.UserHomeDir()
+	if err != nil || home == "" {
+		return false
+	}
+	return samePathDir(dir, filepath.Join(home, ".multica", "hooks"))
+}
+
+func samePathDir(a, b string) bool {
+	absA, err := filepath.Abs(a)
+	if err != nil {
+		return false
+	}
+	absB, err := filepath.Abs(b)
+	if err != nil {
+		return false
+	}
+	absA = filepath.Clean(absA)
+	absB = filepath.Clean(absB)
+	if realA, err := filepath.EvalSymlinks(absA); err == nil {
+		absA = realA
+	}
+	if realB, err := filepath.EvalSymlinks(absB); err == nil {
+		absB = realB
+	}
+	return absA == absB
+}
+
+func canonicalExecutablePath(path string) string {
+	abs, err := filepath.Abs(path)
+	if err != nil {
+		return path
+	}
+	if real, err := filepath.EvalSymlinks(abs); err == nil {
+		return real
+	}
+	return abs
+}
+
+func isExecutableFile(path string) bool {
+	info, err := os.Stat(path)
+	if err != nil || info.IsDir() {
+		return false
+	}
+	return info.Mode()&0o111 != 0
+}
+
 // defaultAgentCommandNames lists the command names the agent probe loop tries
 // before any MULTICA_*_PATH override is applied. Kept in sync with the
 // `probe(...)` calls in LoadConfig — the shell-fallback resolver uses this
@@ -806,7 +898,9 @@ func resolveAgentsViaLoginShell(names []string) map[string]string {
 //     than hand back garbage),
 //  5. canonicalises the directory via `cd ... && pwd -P` so symlinked prefix
 //     dirs (fnm/nvm/volta) collapse to stable paths,
-//  6. prints `<name>\t<canonical_path>` one entry per line for the caller.
+//  6. if the resolved path lives in ~/.multica/hooks, searches the same
+//     shell-expanded PATH for the first executable outside that hooks dir,
+//  7. prints `<name>\t<canonical_path>` one entry per line for the caller.
 //
 // Why steps 2 is important — and why this PR's first revision missed #2512:
 // the motivating case has `alias claude=...` in ~/.zshrc *and* fnm's real
@@ -835,6 +929,18 @@ func buildLoginShellResolveScript(names []string) string {
 	b.WriteString("  [ -n \"$p\" ] || continue\n")
 	b.WriteString("  case \"$p\" in /*) ;; *) continue ;; esac\n")
 	b.WriteString("  d=$(dirname \"$p\") && f=$(basename \"$p\") && c=$(cd \"$d\" 2>/dev/null && pwd -P) || continue\n")
+	b.WriteString("  hc=\"\"\n")
+	b.WriteString("  if [ -n \"${HOME:-}\" ]; then hd=\"$HOME/.multica/hooks\"; hc=$(cd \"$hd\" 2>/dev/null && pwd -P) || hc=\"\"; fi\n")
+	b.WriteString("  if [ -n \"$hc\" ] && [ \"$c\" = \"$hc\" ]; then\n")
+	b.WriteString("    oldIFS=$IFS; IFS=:\n")
+	b.WriteString("    for d2 in $PATH; do\n")
+	b.WriteString("      [ -n \"$d2\" ] || d2=.\n")
+	b.WriteString("      c2=$(cd \"$d2\" 2>/dev/null && pwd -P) || continue\n")
+	b.WriteString("      [ \"$c2\" = \"$hc\" ] && continue\n")
+	b.WriteString("      if [ -f \"$c2/$n\" ] && [ -x \"$c2/$n\" ]; then c=\"$c2\"; f=\"$n\"; break; fi\n")
+	b.WriteString("    done\n")
+	b.WriteString("    IFS=$oldIFS\n")
+	b.WriteString("  fi\n")
 	b.WriteString("  printf '%s\\t%s\\n' \"$n\" \"$c/$f\"\n")
 	b.WriteString("done\n")
 	return b.String()
--- a/server/internal/daemon/config_test.go
+++ b/server/internal/daemon/config_test.go
@@ -243,6 +243,124 @@ func stageFakeAgent(t *testing.T) string {
 	return binDir
 }

+func TestLoadConfig_SkipsMulticaHooksShadowingAgentBinaries(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("POSIX shell not available on Windows")
+	}
+
+	home := t.TempDir()
+	t.Setenv("HOME", home)
+	hooksDir := filepath.Join(home, ".multica", "hooks")
+	if err := os.MkdirAll(hooksDir, 0o755); err != nil {
+		t.Fatalf("create hooks dir: %v", err)
+	}
+	realBinDir := t.TempDir()
+
+	for _, name := range []string{"claude", "codex", "hermes"} {
+		hookPath := filepath.Join(hooksDir, name)
+		hookBody := "#!/bin/sh\nexec " + name + " \"$@\"\n"
+		if err := os.WriteFile(hookPath, []byte(hookBody), 0o755); err != nil {
+			t.Fatalf("write hook wrapper %s: %v", name, err)
+		}
+		realPath := filepath.Join(realBinDir, name)
+		if err := os.WriteFile(realPath, []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil {
+			t.Fatalf("write real binary %s: %v", name, err)
+		}
+	}
+
+	t.Setenv("PATH", hooksDir+string(os.PathListSeparator)+realBinDir)
+	t.Setenv("SHELL", filepath.Join(t.TempDir(), "fish"))
+	t.Setenv("MULTICA_DAEMON_ID", "11111111-1111-1111-1111-111111111111")
+
+	cfg, err := LoadConfig(Overrides{
+		ServerURL:      "http://localhost:0",
+		WorkspacesRoot: t.TempDir(),
+	})
+	if err != nil {
+		t.Fatalf("LoadConfig: %v", err)
+	}
+
+	for provider, binary := range map[string]string{
+		"claude": "claude",
+		"codex":  "codex",
+		"hermes": "hermes",
+	} {
+		got, ok := cfg.Agents[provider]
+		if !ok {
+			t.Fatalf("expected %s agent in config, got %#v", provider, cfg.Agents)
+		}
+		want := canonicalExecutablePath(filepath.Join(realBinDir, binary))
+		if got.Path != want {
+			t.Errorf("%s path = %q, want unshadowed real binary %q", provider, got.Path, want)
+		}
+		if strings.HasPrefix(got.Path, hooksDir) {
+			t.Errorf("%s path still points into hooks dir: %q", provider, got.Path)
+		}
+	}
+}
+
+func TestLoadConfig_SkipsMulticaHooksFromLoginShellFallback(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("POSIX shell not available on Windows")
+	}
+	sh := "/bin/sh"
+	if _, err := os.Stat(sh); err != nil {
+		t.Skipf("no /bin/sh available: %v", err)
+	}
+
+	home := t.TempDir()
+	t.Setenv("HOME", home)
+	hooksDir := filepath.Join(home, ".multica", "hooks")
+	if err := os.MkdirAll(hooksDir, 0o755); err != nil {
+		t.Fatalf("create hooks dir: %v", err)
+	}
+	realBinDir := t.TempDir()
+	hookPath := filepath.Join(hooksDir, "codex")
+	if err := os.WriteFile(hookPath, []byte("#!/bin/sh\nexec codex \"$@\"\n"), 0o755); err != nil {
+		t.Fatalf("write hook wrapper: %v", err)
+	}
+	realPath := filepath.Join(realBinDir, "codex")
+	if err := os.WriteFile(realPath, []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil {
+		t.Fatalf("write real codex: %v", err)
+	}
+
+	t.Setenv("PATH", "/usr/bin:/bin")
+	if _, err := exec.LookPath("codex"); err == nil {
+		t.Skip("PATH leak - codex already visible to daemon without shell fallback")
+	}
+	rc := filepath.Join(t.TempDir(), "sh.rc")
+	rcBody := "export PATH=\"" + hooksDir + string(os.PathListSeparator) + realBinDir + ":$PATH\"\n"
+	if err := os.WriteFile(rc, []byte(rcBody), 0o644); err != nil {
+		t.Fatalf("write shell rc: %v", err)
+	}
+	t.Setenv("SHELL", sh)
+	t.Setenv("ENV", rc)
+	t.Setenv("MULTICA_DAEMON_ID", "11111111-1111-1111-1111-111111111111")
+	pinNonCodexAgentsToMissingPaths(t)
+	oldBundlePaths := codexDesktopAppBundlePaths
+	codexDesktopAppBundlePaths = func() []string { return nil }
+	t.Cleanup(func() { codexDesktopAppBundlePaths = oldBundlePaths })
+
+	cfg, err := LoadConfig(Overrides{
+		ServerURL:      "http://localhost:0",
+		WorkspacesRoot: t.TempDir(),
+	})
+	if err != nil {
+		t.Fatalf("LoadConfig: %v", err)
+	}
+	got, ok := cfg.Agents["codex"]
+	if !ok {
+		t.Fatalf("expected codex from login-shell fallback, got %#v", cfg.Agents)
+	}
+	want := canonicalExecutablePath(realPath)
+	if got.Path != want {
+		t.Fatalf("codex path = %q, want unshadowed real binary %q", got.Path, want)
+	}
+	if strings.HasPrefix(got.Path, hooksDir) {
+		t.Fatalf("codex path still points into hooks dir: %q", got.Path)
+	}
+}
+
 // TestLoadConfig_AutoUpdateDefault_SelfHostOff is the regression guard for
 // MUL-2381: a daemon pointed at any non-cloud server URL must default
 // AutoUpdateEnabled to false, because self-host operators frequently run a