diff --git a/server/internal/daemon/agent_path_selfheal_test.go b/server/internal/daemon/agent_path_selfheal_test.go new file mode 100644 index 000000000..6edbea5f9 --- /dev/null +++ b/server/internal/daemon/agent_path_selfheal_test.go @@ -0,0 +1,321 @@ +package daemon + +import ( + "context" + "log/slog" + "os" + "path/filepath" + "runtime" + "strings" + "testing" +) + +// newSelfHealTestDaemon builds a Daemon with just the state resolveAgentEntry +// touches (logger + the two maps it reads/writes), so these tests don't need a +// fake HTTP server. healGroup / the mutexes are usable at their zero value. +func newSelfHealTestDaemon() *Daemon { + return &Daemon{ + logger: slog.Default(), + resolvedPaths: make(map[string]healedAgent), + agentVersions: make(map[string]string), + } +} + +// stubDetectVersionFromPath makes detectAgentVersion report the version encoded +// in an installVersionedCodex path (…/codex//bin/codex), so a heal to a +// v2 directory "detects" v2. checkAgentMinVersion is intentionally left as the +// real agent.CheckMinVersion so the min-version gate is exercised for real. +func stubDetectVersionFromPath(t *testing.T) { + t.Helper() + orig := detectAgentVersion + detectAgentVersion = func(_ context.Context, path string) (string, error) { + return filepath.Base(filepath.Dir(filepath.Dir(path))), nil + } + t.Cleanup(func() { detectAgentVersion = orig }) +} + +// writeExecStub writes a runnable no-op executable at path, creating parents. +func writeExecStub(t *testing.T, path string) { + t.Helper() + if err := os.MkdirAll(filepath.Dir(path), 0o755); err != nil { + t.Fatalf("mkdir for stub %s: %v", path, err) + } + if err := os.WriteFile(path, []byte("#!/bin/sh\nexit 0\n"), 0o755); err != nil { + t.Fatalf("write stub %s: %v", path, err) + } +} + +// installVersionedCodex lays out a version-manager style install under root: +// a concrete versioned binary plus a stable-name symlink pointing at it (the +// shape Homebrew Cask / nvm / fnm produce). It returns the canonical path the +// daemon would pin for the stable name, and repoints the symlink atomically on +// later calls to simulate an in-place upgrade. +func installVersionedCodex(t *testing.T, root, version, stableBin string) string { + t.Helper() + versioned := filepath.Join(root, "Caskroom", "codex", version, "bin", "codex") + writeExecStub(t, versioned) + link := filepath.Join(stableBin, "codex") + _ = os.Remove(link) // repoint on upgrade + if err := os.MkdirAll(stableBin, 0o755); err != nil { + t.Fatalf("mkdir stable bin: %v", err) + } + if err := os.Symlink(versioned, link); err != nil { + t.Fatalf("symlink %s -> %s: %v", link, versioned, err) + } + return canonicalExecutablePath(link) +} + +// TestResolveAgentEntry_SelfHealsAfterInPlaceUpgrade reproduces MUL-4486: a +// version manager upgrades codex in place, deleting the versioned directory the +// daemon pinned at startup and repointing the stable command name at the new +// version. resolveAgentEntry must re-resolve the pinned path AND return the +// matching version, so the caller keys downstream policy off the binary that +// will actually run. +func TestResolveAgentEntry_SelfHealsAfterInPlaceUpgrade(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink/exec-bit layout is POSIX-specific") + } + stubDetectVersionFromPath(t) + + root := t.TempDir() + stableBin := filepath.Join(root, "bin") // the stable "/opt/homebrew/bin" analogue + t.Setenv("PATH", stableBin) + // Pin resolution to the daemon's own PATH: an unsupported shell disables the + // login-shell fallback so the test can't accidentally resolve a real codex + // installed on the host running it. + t.Setenv("SHELL", filepath.Join(t.TempDir(), "fish")) + + v1 := installVersionedCodex(t, root, "0.144.1", stableBin) + if !strings.Contains(v1, "0.144.1") { + t.Fatalf("pinned path %q does not point into the v1 versioned dir", v1) + } + + d := newSelfHealTestDaemon() + d.setAgentVersion("codex", "0.144.1") // the version detected for v1 at startup + entry := AgentEntry{Path: v1, Command: "codex"} + ctx := context.Background() + + // While the pinned binary is present it must be returned unchanged, paired + // with its registration-detected version — the anti-PATH-redirect case. + if got, ver := d.resolveAgentEntry(ctx, "codex", entry); got.Path != v1 || ver != "0.144.1" { + t.Fatalf("live pinned path/version rewritten: got (%q, %q), want (%q, %q)", got.Path, ver, v1, "0.144.1") + } + + // In-place upgrade: drop the v1 tree and repoint the stable symlink at v2. + if err := os.RemoveAll(filepath.Join(root, "Caskroom", "codex", "0.144.1")); err != nil { + t.Fatalf("remove v1 tree: %v", err) + } + if agentExecutablePresent(v1) { + t.Fatalf("v1 path still present after removing its tree: %q", v1) + } + v2 := installVersionedCodex(t, root, "0.144.3", stableBin) + + got, ver := d.resolveAgentEntry(ctx, "codex", entry) + if got.Path != v2 { + t.Fatalf("self-heal resolved %q, want re-resolved v2 %q", got.Path, v2) + } + if !agentExecutablePresent(got.Path) { + t.Fatalf("self-healed path is not runnable: %q", got.Path) + } + // Must-fix (MUL-4486 review): the version returned is paired with the healed + // path, and the shared version cache moved with it too. + if ver != "0.144.3" { + t.Fatalf("returned version not paired with healed path: got %q, want %q", ver, "0.144.3") + } + if v := d.agentVersion("codex"); v != "0.144.3" { + t.Fatalf("version cache not updated in lockstep: got %q, want %q", v, "0.144.3") + } + + // A subsequent call with the same stale entry must reuse the remembered + // {path, version} without re-resolving. Prove it by breaking PATH: if it + // re-resolved now it would miss, but the cached heal still resolves. + t.Setenv("PATH", filepath.Join(root, "empty")) + if got, ver := d.resolveAgentEntry(ctx, "codex", entry); got.Path != v2 || ver != "0.144.3" { + t.Fatalf("cached self-heal not reused: got (%q, %q), want (%q, %q)", got.Path, ver, v2, "0.144.3") + } +} + +// TestResolveAgentEntry_RejectsBelowMinVersionAfterUpgrade covers the review's +// concern: if re-resolution lands on a build below the minimum supported +// version, it must NOT be adopted or launched, and the cached version must not +// be corrupted downward. +func TestResolveAgentEntry_RejectsBelowMinVersionAfterUpgrade(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink/exec-bit layout is POSIX-specific") + } + stubDetectVersionFromPath(t) // checkAgentMinVersion stays real: codex min is 0.100.0 + + root := t.TempDir() + stableBin := filepath.Join(root, "bin") + t.Setenv("PATH", stableBin) + t.Setenv("SHELL", filepath.Join(t.TempDir(), "fish")) + + v1 := installVersionedCodex(t, root, "0.144.1", stableBin) + + d := newSelfHealTestDaemon() + d.setAgentVersion("codex", "0.144.1") + entry := AgentEntry{Path: v1, Command: "codex"} + ctx := context.Background() + + // "Upgrade" actually repoints at a below-minimum build (< 0.100.0). + if err := os.RemoveAll(filepath.Join(root, "Caskroom", "codex", "0.144.1")); err != nil { + t.Fatalf("remove v1 tree: %v", err) + } + installVersionedCodex(t, root, "0.9.0", stableBin) + + got, ver := d.resolveAgentEntry(ctx, "codex", entry) + if got.Path != v1 { + t.Fatalf("below-min build was adopted: got %q, want stale pinned %q", got.Path, v1) + } + // The returned version stays the (stale) base — never the below-min value. + if ver != "0.144.1" { + t.Fatalf("returned version corrupted to below-min: got %q, want %q", ver, "0.144.1") + } + d.resolvedPathsMu.RLock() + _, cached := d.resolvedPaths["codex"] + d.resolvedPathsMu.RUnlock() + if cached { + t.Fatalf("below-min build was cached as a healed path") + } + if v := d.agentVersion("codex"); v != "0.144.1" { + t.Fatalf("cached version was corrupted to a below-min value: got %q, want %q", v, "0.144.1") + } +} + +// TestResolveAgentEntry_ReturnsVersionPairedWithCachedPath is the regression +// for the second-round review race: a task that hits the already-healed path +// (via the resolvedPaths fast path, without entering singleflight) must read +// the version that goes WITH that path, not whatever the separately-mutable +// agentVersions cache happens to hold. We heal to v2, then deliberately skew +// agentVersions to a bogus value to stand in for the window where the two +// caches disagree, and assert the reused resolution still returns v2's version. +func TestResolveAgentEntry_ReturnsVersionPairedWithCachedPath(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink/exec-bit layout is POSIX-specific") + } + stubDetectVersionFromPath(t) + + root := t.TempDir() + stableBin := filepath.Join(root, "bin") + t.Setenv("PATH", stableBin) + t.Setenv("SHELL", filepath.Join(t.TempDir(), "fish")) + + v1 := installVersionedCodex(t, root, "0.144.1", stableBin) + d := newSelfHealTestDaemon() + d.setAgentVersion("codex", "0.144.1") + entry := AgentEntry{Path: v1, Command: "codex"} + ctx := context.Background() + + if err := os.RemoveAll(filepath.Join(root, "Caskroom", "codex", "0.144.1")); err != nil { + t.Fatalf("remove v1 tree: %v", err) + } + v2 := installVersionedCodex(t, root, "0.144.3", stableBin) + + // First call heals and caches {v2, 0.144.3}. + if got, ver := d.resolveAgentEntry(ctx, "codex", entry); got.Path != v2 || ver != "0.144.3" { + t.Fatalf("initial heal wrong: got (%q, %q)", got.Path, ver) + } + // Simulate the two caches transiently disagreeing (the exact window the race + // was about): the shared version cache holds a value that does NOT match the + // cached path. The paired return must still win. + d.setAgentVersion("codex", "0.0.1-stale") + + got, ver := d.resolveAgentEntry(ctx, "codex", entry) + if got.Path != v2 { + t.Fatalf("cached path not reused: got %q, want %q", got.Path, v2) + } + if ver != "0.144.3" { + t.Fatalf("returned version came from the skewed shared cache, not the cached path pairing: got %q, want %q", ver, "0.144.3") + } +} + +// TestResolveAgentEntry_HealedPathWinsOverReappearingPinnedPath covers the +// follow-up edge from the third-round review: after a self-heal to v2, if the +// originally pinned v1 path reappears on disk (a downgrade / reinstall that +// recreates the old versioned directory) while the healed v2 binary is still +// present, resolveAgentEntry must keep returning the healed {v2, its version} +// pair. Returning the reappeared v1 path here would pair the old binary with +// the healed v2 version — the mismatched {old path, new version} the review +// flagged. +func TestResolveAgentEntry_HealedPathWinsOverReappearingPinnedPath(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("symlink/exec-bit layout is POSIX-specific") + } + stubDetectVersionFromPath(t) + + root := t.TempDir() + stableBin := filepath.Join(root, "bin") + t.Setenv("PATH", stableBin) + t.Setenv("SHELL", filepath.Join(t.TempDir(), "fish")) + + v1 := installVersionedCodex(t, root, "0.144.1", stableBin) + d := newSelfHealTestDaemon() + d.setAgentVersion("codex", "0.144.1") + entry := AgentEntry{Path: v1, Command: "codex"} + ctx := context.Background() + + // In-place upgrade: drop v1, repoint the stable symlink at v2, and heal. + if err := os.RemoveAll(filepath.Join(root, "Caskroom", "codex", "0.144.1")); err != nil { + t.Fatalf("remove v1 tree: %v", err) + } + v2 := installVersionedCodex(t, root, "0.144.3", stableBin) + if got, ver := d.resolveAgentEntry(ctx, "codex", entry); got.Path != v2 || ver != "0.144.3" { + t.Fatalf("initial heal wrong: got (%q, %q), want (%q, %q)", got.Path, ver, v2, "0.144.3") + } + + // The originally pinned v1 path reappears (a reinstall recreating the old + // versioned dir) while the healed v2 binary is still present on disk. + writeExecStub(t, v1) + if !agentExecutablePresent(v1) { + t.Fatalf("v1 path should be runnable again after reinstall: %q", v1) + } + + got, ver := d.resolveAgentEntry(ctx, "codex", entry) + if got.Path != v2 { + t.Fatalf("reappearing pinned path hijacked the launch: got %q, want healed %q", got.Path, v2) + } + if ver != "0.144.3" { + t.Fatalf("returned version not paired with healed path: got %q, want %q", ver, "0.144.3") + } +} + +// TestResolveAgentEntry_UninstalledLeavesEntryUnchanged verifies that when the +// binary is genuinely gone (not just moved by an upgrade), resolveAgentEntry +// returns the entry untouched so the downstream "executable not found" error +// still surfaces rather than being silently swallowed. +func TestResolveAgentEntry_UninstalledLeavesEntryUnchanged(t *testing.T) { + if runtime.GOOS == "windows" { + t.Skip("POSIX-specific layout") + } + + root := t.TempDir() + stableBin := filepath.Join(root, "bin") + t.Setenv("PATH", stableBin) + // Disable the login-shell fallback so an actual codex on the host running + // this test can't stand in for the "uninstalled" binary. + t.Setenv("SHELL", filepath.Join(t.TempDir(), "fish")) + pinned := installVersionedCodex(t, root, "0.144.1", stableBin) + + // Uninstall entirely: remove the versioned tree and the stable symlink. + if err := os.RemoveAll(root); err != nil { + t.Fatalf("remove install root: %v", err) + } + + d := newSelfHealTestDaemon() + entry := AgentEntry{Path: pinned, Command: "codex"} + got, _ := d.resolveAgentEntry(context.Background(), "codex", entry) + if got.Path != pinned { + t.Fatalf("expected entry unchanged when binary is gone, got %q want %q", got.Path, pinned) + } +} + +// TestResolveAgentEntry_NoCommandNoHeal verifies that a synthesized entry with +// no recorded Command (e.g. a custom runtime profile carrying an absolute path) +// is never re-resolved: the entry is returned as-is even when its path is gone. +func TestResolveAgentEntry_NoCommandNoHeal(t *testing.T) { + d := newSelfHealTestDaemon() + entry := AgentEntry{Path: filepath.Join(t.TempDir(), "does-not-exist"), Command: ""} + if got, _ := d.resolveAgentEntry(context.Background(), "codex", entry); got.Path != entry.Path { + t.Fatalf("entry with empty Command was rewritten: got %q, want %q", got.Path, entry.Path) + } +} diff --git a/server/internal/daemon/config.go b/server/internal/daemon/config.go index 3e35f8898..b02cc6128 100644 --- a/server/internal/daemon/config.go +++ b/server/internal/daemon/config.go @@ -230,8 +230,9 @@ func LoadConfig(overrides Overrides) (Config, error) { cmd := envOrDefault(envVar, defaultCmd) if path, err := resolveAgentExecutablePath(cmd); err == nil { return AgentEntry{ - Path: path, - Model: strings.TrimSpace(os.Getenv(modelEnv)), + Path: path, + Command: cmd, + Model: strings.TrimSpace(os.Getenv(modelEnv)), }, true } // The shell fallback only rescues bare command names. An operator @@ -243,8 +244,9 @@ func LoadConfig(overrides Overrides) (Config, error) { } if path, ok := getShellResolved()[cmd]; ok { return AgentEntry{ - Path: path, - Model: strings.TrimSpace(os.Getenv(modelEnv)), + Path: path, + Command: cmd, + Model: strings.TrimSpace(os.Getenv(modelEnv)), }, true } if defaultCmd == "codex" && cmd == defaultCmd { @@ -253,8 +255,9 @@ func LoadConfig(overrides Overrides) (Config, error) { for _, p := range codexDesktopAppBundlePaths() { if _, err := os.Stat(p); err == nil { return AgentEntry{ - Path: p, - Model: strings.TrimSpace(os.Getenv(modelEnv)), + Path: p, + Command: cmd, + Model: strings.TrimSpace(os.Getenv(modelEnv)), }, true } } @@ -306,8 +309,9 @@ func LoadConfig(overrides Overrides) (Config, error) { qoderPath := envOrDefault("MULTICA_QODER_PATH", "qodercli") if path, err := resolveAgentExecutablePath(qoderPath); err == nil { agents["qoder"] = AgentEntry{ - Path: path, - Model: strings.TrimSpace(os.Getenv("MULTICA_QODER_MODEL")), + Path: path, + Command: qoderPath, + Model: strings.TrimSpace(os.Getenv("MULTICA_QODER_MODEL")), } } // ByteDance official TRAE CLI (the `traecli` binary from https://docs.trae.cn/cli), @@ -694,6 +698,47 @@ func resolveAgentExecutablePath(cmd string) (string, error) { return canonicalExecutablePath(resolved), nil } +// agentExecutablePresent reports whether path currently resolves to a runnable +// executable, using the exact check the agent backends apply at launch +// (exec.LookPath). A pinned AgentEntry.Path that fails this has vanished from +// disk — typically because a version manager did an in-place upgrade and +// deleted the old versioned directory the path pointed into (MUL-4486). +func agentExecutablePresent(path string) bool { + if path == "" { + return false + } + _, err := exec.LookPath(path) + return err == nil +} + +// reresolveAgentCommand re-runs the startup resolution for a single agent +// command name, returning the freshly resolved absolute path. It mirrors the +// probe() order in LoadConfig: exec.LookPath (with the ~/.multica/hooks +// exclusion preserved via resolveAgentExecutablePath) first, then the login +// shell fallback for a bare command name a GUI-launched daemon can't see on +// its own PATH. It is only called on the miss path — when a previously pinned +// path has disappeared — so the login-shell cost is paid rarely, never on a +// normal launch. +func reresolveAgentCommand(cmd string) (string, bool) { + if cmd == "" { + return "", false + } + if path, err := resolveAgentExecutablePath(cmd); err == nil { + return path, true + } + // A bare command name the daemon's own PATH can't see: retry via the + // user's login shell, exactly as the startup probe does for + // fnm/nvm/native-installer prefixes. Absolute/relative overrides skip + // this — an operator-pinned MULTICA_*_PATH that no longer exists should + // stay a hard miss rather than silently resolve a different binary. + if !strings.ContainsAny(cmd, "/\\") { + if path, ok := resolveAgentsViaLoginShell([]string{cmd})[cmd]; ok { + return path, true + } + } + return "", false +} + func lookPathExcludingMulticaHooks(cmd string) (string, error) { for _, dir := range filepath.SplitList(os.Getenv("PATH")) { if dir == "" { diff --git a/server/internal/daemon/daemon.go b/server/internal/daemon/daemon.go index 71883f933..24b0864f0 100644 --- a/server/internal/daemon/daemon.go +++ b/server/internal/daemon/daemon.go @@ -18,6 +18,8 @@ import ( "sync/atomic" "time" + "golang.org/x/sync/singleflight" + "github.com/multica-ai/multica/server/internal/cli" "github.com/multica-ai/multica/server/internal/daemon/execenv" "github.com/multica-ai/multica/server/internal/daemon/repocache" @@ -172,6 +174,23 @@ type Daemon struct { versionsMu sync.RWMutex // guards agentVersions agentVersions map[string]string // provider -> detected CLI version (set during registration) + // resolvedPathsMu guards resolvedPaths, the self-healed executable paths. + // The daemon pins each agent's absolute path at startup so a later PATH + // change can't redirect a task launch. When that pinned path later vanishes + // (a version manager did an in-place upgrade — Homebrew Cask, nvm/fnm), + // resolveAgentEntry re-resolves the original command once and records the + // result here so subsequent launches, model lists, and registrations reuse + // it without re-resolving. Path and detected version are stored together so + // any reader that observes the new path also observes the matching version + // (no "new binary under stale version policy" window). Keyed by provider. + // See MUL-4486. + resolvedPathsMu sync.RWMutex + resolvedPaths map[string]healedAgent + // healGroup coalesces concurrent self-heal re-resolutions per provider so a + // just-upgraded agent seen by many queued tasks at once pays for a single + // login-shell probe + version detection instead of one per task (MUL-4486). + healGroup singleflight.Group + wsHBMu sync.RWMutex // guards wsHBLastAck wsHBLastAck map[string]time.Time // runtime_id -> last successful WS heartbeat ack timestamp @@ -281,6 +300,7 @@ func New(cfg Config, logger *slog.Logger) *Daemon { profileLaunchSpecs: make(map[string]profileLaunchSpec), runtimeSet: newRuntimeSetWatcher(), agentVersions: make(map[string]string), + resolvedPaths: make(map[string]healedAgent), wsHBLastAck: make(map[string]time.Time), activeEnvRoots: make(map[string]int), localPathLocks: NewLocalPathLocker(), @@ -313,6 +333,150 @@ func (d *Daemon) agentVersion(provider string) string { return d.agentVersions[provider] } +// healedAgent bundles a self-healed executable path with the CLI version +// detected for it. The two are published together under resolvedPathsMu and +// returned together from resolveAgentEntry, so a caller that observes the new +// path necessarily observes the matching version — closing the window where a +// just-upgraded binary would run under the daemon's previous version policy +// (MUL-4486 review). +type healedAgent struct { + path string + version string +} + +// resolveAgentEntry returns entry with a usable executable path plus the CLI +// version that corresponds to that path, self-healing the pinned Path when it +// has vanished from disk (MUL-4486). +// +// The daemon pins each agent's absolute, symlink-resolved path at startup so a +// later PATH change cannot redirect a task launch. But a version manager +// (Homebrew Cask, nvm/fnm) upgrading in place deletes the old versioned +// directory that pinned path points into and repoints the stable command name +// at the new version — leaving the daemon holding a path that no longer exists +// until it restarts. Every consumer of entry.Path (task launch, model listing, +// version detection at registration) then hard-fails with "executable not +// found". +// +// The returned version always matches the returned path, so callers key +// version-sensitive policy (e.g. the Codex sandbox) off it directly rather than +// re-reading the shared version cache, which a concurrent heal could still be +// updating. +// +// Behaviour: +// - A previous self-heal that is still live -> returned with its paired +// version. This is checked first so that once we've re-resolved to a new +// binary, a reappearing stale path (a downgrade / reinstall recreating the +// old versioned directory) cannot re-pair that old binary with the healed +// version — a mismatched {old path, new version} (MUL-4486 review). +// - Otherwise, pinned Path still present -> returned unchanged, paired with +// its registration-detected version. The anti-redirect guarantee holds for +// the normal (never-healed) case: a live pinned binary is never +// second-guessed even if PATH now points elsewhere. +// - Pinned Path gone and no live heal -> re-resolve entry.Command once +// (preserving the ~/.multica/hooks exclusion and the login-shell fallback). +// Before adopting the re-resolved binary it is version-detected and run +// through the same minimum-version gate registration applies. This +// reproduces exactly what a daemon restart would resolve, so it is no less +// safe than the documented restart workaround — only automatic. +// - Re-resolution fails, the candidate can't be version-detected, or it is +// below the minimum supported version -> entry is returned unchanged so the +// candidate is never launched and the downstream error still surfaces. +func (d *Daemon) resolveAgentEntry(ctx context.Context, provider string, entry AgentEntry) (AgentEntry, string) { + // A prior self-heal wins over the original pinned path: it carries a + // {path, version} pair we already verified together, so it can never regress + // to the mismatched pairing a reappearing stale path would produce. + d.resolvedPathsMu.RLock() + healed, ok := d.resolvedPaths[provider] + d.resolvedPathsMu.RUnlock() + if ok && agentExecutablePresent(healed.path) { + entry.Path = healed.path + return entry, healed.version + } + + if agentExecutablePresent(entry.Path) { + return entry, d.agentVersion(provider) + } + + if entry.Command == "" { + return entry, d.agentVersion(provider) + } + + // Coalesce concurrent heals for the same provider: the first task through + // pays for the re-resolve + version detection, the rest share its result + // instead of each spawning their own login shell and `--version` probe. + command := entry.Command + v, _, _ := d.healGroup.Do(provider, func() (any, error) { + return d.healAgentPath(ctx, provider, command), nil + }) + healed, _ = v.(healedAgent) + if healed.path == "" { + return entry, d.agentVersion(provider) + } + entry.Path = healed.path + return entry, healed.version +} + +// healAgentPath re-resolves command for provider and, if a usable binary is +// found, records it and returns it. "Usable" means: it resolves, its version +// can be detected, and that version meets the same minimum-version gate +// registration enforces. Path and version are published together under +// resolvedPathsMu so any observer of the path also sees the matching version; +// the shared d.agentVersion cache is refreshed too, for registration hygiene. +// It returns a zero healedAgent when nothing usable was found, so the caller +// keeps the (stale) pinned entry and the candidate is never launched. Runs +// under healGroup, one invocation at a time per provider. +func (d *Daemon) healAgentPath(ctx context.Context, provider, command string) healedAgent { + // Re-check the cache: a predecessor under the same singleflight key may have + // already populated it, or a prior heal completed between the read above and + // entering here. + d.resolvedPathsMu.RLock() + cached, ok := d.resolvedPaths[provider] + d.resolvedPathsMu.RUnlock() + if ok && agentExecutablePresent(cached.path) { + return cached + } + + newPath, found := reresolveAgentCommand(command) + if !found { + return healedAgent{} + } + + // Verify before adopting. An in-place "upgrade" that actually repoints at an + // older or broken install must not be launched under the daemon's stale + // version policy, and must not slip past the minimum-version gate that the + // registration path applies (MUL-4486 review). + version, err := detectAgentVersion(ctx, newPath) + if err != nil { + d.logger.Warn("re-resolved agent executable failed version detection; keeping pinned path", + "provider", provider, "command", command, "new_path", newPath, "error", err) + return healedAgent{} + } + if err := checkAgentMinVersion(provider, version); err != nil { + d.logger.Warn("re-resolved agent executable is below the minimum supported version; not adopting it", + "provider", provider, "command", command, "new_path", newPath, "version", version, "error", err) + return healedAgent{} + } + + adopted := healedAgent{path: newPath, version: version} + // Publish path + version atomically: any reader that sees the new path in + // resolveAgentEntry gets the matching version out of the same struct value. + d.resolvedPathsMu.Lock() + if d.resolvedPaths == nil { + d.resolvedPaths = make(map[string]healedAgent) + } + d.resolvedPaths[provider] = adopted + d.resolvedPathsMu.Unlock() + // Keep the registration version cache fresh too. The task path reads the + // version returned alongside the resolved path (above), not this map, so its + // staleness can never gate a launch — this is hygiene for the registration + // report and any future d.agentVersion reader. + d.setAgentVersion(provider, version) + + d.logger.Info("re-resolved agent executable after pinned path vanished (in-place upgrade)", + "provider", provider, "command", command, "new_path", newPath, "version", version) + return adopted +} + func (d *Daemon) notifyRuntimeSetChanged() { d.runtimeSet.notify() } @@ -949,6 +1113,12 @@ func (d *Daemon) registerRuntimesForWorkspace(ctx context.Context, workspaceID s var runtimes []map[string]string var failedProfiles []map[string]string for name, entry := range d.cfg.Agents { + // Self-heal a pinned executable path an in-place upgrade deleted + // (MUL-4486) so version detection — and thus staying registered/online + // — recovers without a daemon restart. resolveAgentEntry already + // version-gates the healed binary; the detect + min-version check below + // still runs to produce the version string this registration reports. + entry, _ = d.resolveAgentEntry(ctx, name, entry) version, err := detectAgentVersion(ctx, entry.Path) if err != nil { d.logger.Warn("skip registering runtime", "name", name, "error", err) @@ -2124,6 +2294,8 @@ func (d *Daemon) handleModelList(ctx context.Context, rt Runtime, requestID stri }) return } + // Self-heal a pinned executable path an in-place upgrade deleted (MUL-4486). + entry, _ = d.resolveAgentEntry(ctx, rt.Provider, entry) models, err := agent.ListModels(ctx, rt.Provider, entry.Path) if err != nil { @@ -3515,6 +3687,10 @@ func (d *Daemon) runTask(ctx context.Context, task Task, provider string, slot i // agent of the same provider installed, so when the runtime is custom we // synthesize an AgentEntry instead of hard-failing on the !ok lookup. var profileFixedArgs []string + // resolvedVersion is the CLI version of the built-in binary entry.Path + // resolves to, paired with the path by resolveAgentEntry so a just-upgraded + // codex is never launched under the previous version's policy (MUL-4486). + var resolvedVersion string if customSpec, isCustom := d.customProfileLaunchForRuntime(task.RuntimeID); isCustom { entry.Path = customSpec.path profileFixedArgs = customSpec.fixedArgs @@ -3523,6 +3699,12 @@ func (d *Daemon) runTask(ctx context.Context, task Task, provider string, slot i "task_id", task.ID, "runtime_id", task.RuntimeID, "provider", provider, "command_path", customSpec.path, "fixed_args", len(profileFixedArgs)) + } else if ok { + // Built-in provider: self-heal a pinned executable path that an in-place + // upgrade deleted (MUL-4486). Only reached when no custom profile owns + // the launch, so a custom runtime's path is never second-guessed and a + // custom-only host pays no wasted re-resolution. + entry, resolvedVersion = d.resolveAgentEntry(ctx, provider, entry) } if !ok { return TaskResult{}, fmt.Errorf("no agent configured for provider %q", provider) @@ -3603,7 +3785,16 @@ func (d *Daemon) runTask(ctx context.Context, task Task, provider string, slot i // Try to reuse the workdir from a previous task on the same (agent, issue) pair. var env *execenv.Environment + // For a built-in codex task, use the version paired with the resolved path + // so an in-place upgrade can't leave the sandbox policy on the old version + // (MUL-4486). A custom codex runtime skips the self-heal, so resolvedVersion + // is empty and it keeps the existing cached-version fallback — its binary is + // the profile's own command, which the daemon never pins or version-detects. + // Non-codex providers carry the value through without consuming it. codexVersion := d.agentVersion("codex") + if provider == "codex" && resolvedVersion != "" { + codexVersion = resolvedVersion + } openclawBin := "" if provider == "openclaw" { openclawBin = entry.Path diff --git a/server/internal/daemon/types.go b/server/internal/daemon/types.go index 00b459d67..49b038f2f 100644 --- a/server/internal/daemon/types.go +++ b/server/internal/daemon/types.go @@ -8,8 +8,16 @@ import ( // AgentEntry describes a single available agent CLI. type AgentEntry struct { - Path string // path to CLI binary - Model string // model override (optional) + Path string // path to CLI binary (pinned at startup; symlink-resolved to a concrete, possibly versioned, path) + // Command is the bare command name or MULTICA_*_PATH value that Path was + // resolved from at startup. It is kept so the daemon can re-resolve Path + // if the pinned executable later vanishes — e.g. a version manager + // (Homebrew Cask, nvm/fnm) does an in-place upgrade that deletes the old + // versioned directory Path points into. Empty for synthesized entries + // (custom runtime profiles) that carry an absolute path directly. See + // Daemon.resolveAgentEntry and MUL-4486. + Command string + Model string // model override (optional) } // Runtime represents a registered daemon runtime.