fix(daemon): repair PATH with a tool floor at startup so headless daemons find tmux (#2812)

[Pritom14]

What

Fixes #2812. A daemon started outside the Electron launcher (headless ao start, systemd, cron, container, automation) inherits a minimal $PATH and can't see tmux under /opt/homebrew/bin or /usr/local/bin, so every spawn fails the prereq gate with tmux required on macOS/Linux but not in PATH — even though tmux is installed.

Root cause

session_manager.validateRuntimePrerequisites gates each spawn on exec.LookPath("tmux") against the daemon's own inherited PATH (manager.go:2488 / prod wiring manager.go:228). The PATH floor + login-shell probe that fixes this exists only in the Electron launcher (frontend/src/shared/shell-env.ts), so it never reaches a non-Electron daemon.

Fix

• New internal/runtimeenv package ports the static PATH floor (FALLBACK_PATH_DIRS) from shell-env.ts to Go.
• daemon.Run() calls runtimeenv.EnsureFallbackPath() once at startup, before any exec.LookPath, so the tmux prereq gate and the agent-binary preflight both see the floor.
• Existing PATH entries keep precedence (only appended, de-duped). No-op on Windows (the tmux gate is Windows-exempt; floor dirs are POSIX).

Scope

This is the core fix (#1 in the issue). The two complementary items — enriching ao doctor's tmux check to stat floor dirs, and making the spawn error actionable — are intentionally left as separate follow-up PRs so this stays focused and low-risk.

Tests

internal/runtimeenv/pathfloor_test.go: appends missing floor dirs, preserves existing order/precedence, de-dupes, empty-PATH → floor only, drops empty segments. go build / go vet / go test ./internal/runtimeenv/ all pass; gofmt clean.

🤖 Generated with Claude Code

[i-trytoohard]

Review — PR #2815 (fixes #2812)

Technical review from the issue author. Per repo policy I'm posting this as comment, not approval — final sign-off should come from an authorized maintainer (@yyovil / @dhruv / @Priyanchew et al.).

Could not run go build/go vet/go test myself — no Go toolchain on this box. Findings below are from reading the diff + call-graph tracing. Author claims green build/vet/test; please confirm in CI.

What this does (confirmed correct)

Ports the static PATH floor (FALLBACK_PATH_DIRS) from frontend/src/shared/shell-env.ts into a new internal/runtimeenv package, and calls runtimeenv.EnsureFallbackPath() as the first statement in daemon.Run() (daemon.go:46). The floor appends missing tool dirs to os.Getenv("PATH"), preserving existing precedence and de-duping. No-op on Windows.

Mechanism verified: validateRuntimePrerequisites → m.lookPath("tmux") → exec.LookPath reads os.Getenv("PATH") at call time. Since EnsureFallbackPath mutates PATH via os.Setenv before any spawn request, the repaired PATH is visible to the gate. ✅ This resolves the core reported bug (Homebrew tmux invisible to a headless daemon).

Scope hygiene: clean — 3 files, single commit, no drive-by changes, package doc references the issue. ✅

⚠️ Significant: the floor is wired into only one of two processes that do exec.LookPath("tmux")

EnsureFallbackPath() is called exclusively in daemon.Run() (daemon.go:46, reached only via the hidden ao daemon subcommand at root.go:275). ao doctor runs in a separate CLI process — commandContext.deps.LookPath is wired straight to exec.LookPath at root.go:90, and the CLI PersistentPreRunE (root.go:165) never calls EnsureFallbackPath().

Consequence: after this PR merges, on a headless/systemd box where the daemon now correctly spawns (floor applied), ao doctor will still report tmux: not found in PATH; required on macOS/Linux to start sessions — the identical false positive, because doctor's CLI process never gets the floor.

This matters because my issue's fix #2 was explicitly about doctor confirming the failure instead of diagnosing it. The PR body says doctor is "intentionally left as separate follow-up PRs" — that's a fine scope call for this PR, but the follow-up needs to know that the fix is not "enrich the message" — it's "call EnsureFallbackPath() in the CLI entry too" (e.g., in PersistentPreRunE, or at the top of runDoctor). Worth a // TODO(#2812 follow-up) note on the daemon call site so the next contributor doesn't assume the daemon-only wiring is the whole story. Details in the inline comment on daemon.go:46.

⚠️ Scope limitation worth stating explicitly: static floor only, no login-shell probe

shell-env.ts does two things for the Electron-launched daemon: (a) the static FALLBACK_PATH_DIRS floor, and (b) a login-shell probe ($SHELL -ilc → env -0, via resolveShellEnv) that recovers the user's actual login PATH. This PR ports only (a).

The static floor covers the reported bug (Homebrew at /opt/homebrew/bin, /usr/local/bin) — that's the common case and the PR resolves it. But it does not cover:

• Nix/NixOS — tmux at ~/.nix-profile/bin or /run/wrappers/bin (relevant: at least one core contributor runs NixOS).
• asdf/mise/rtx users with shims in ~/.asdf/shims etc.
• Any non-standard install prefix.

Electron's probe handles all of these; the Go daemon still won't, post-PR. This is an acceptable scope decision for "core fix #1" (and the PR body doesn't claim parity with Electron), but the limitation should be acknowledged — either in the package doc or a follow-up issue — so it's clear the daemon remains less PATH-robust than the Electron path for non-Homebrew setups.

✅ Positive side effect to be aware of

Because EnsureFallbackPath mutates os.Getenv("PATH") process-wide and HookPATH (manager.go:2085) reads os.Getenv("PATH") as the base for session PATH, spawned sessions will now also inherit the floor. This is desirable (sessions need to resolve tmux/git/agent binaries too) and not a regression — but it is a behavior change the PR description doesn't mention. Worth a one-line note in the PR body.

Minor

• pathfloor_test.go tests WithFallbackPath thoroughly (append / precedence / dedupe / empty / empty-segments — good cases) but there's no test that EnsureFallbackPath() actually mutates os.Getenv("PATH"). It's a 3-line wrapper so the risk is low, but since it's the actual fix hook, a tiny test (EnsureFallbackPath(); if !strings.Contains(os.Getenv("PATH"), "/opt/homebrew/bin") { t.Fatal(...) }, with PATH save/restore) would be cheap defensive coverage. (Skip on Windows or guard with runtime.GOOS.)
• os.Setenv/os.Getenv aren't concurrency-safe in Go, but this runs once at startup before any goroutine — fine in practice. No action needed; mentioning for completeness.

Verdict

Comment — no blocking issues for the spawn-gate fix itself. The core mechanism is correct, well-tested, and cleanly scoped. Two things I'd like addressed before merge (maintainer's call on whether they're merge-blocking or follow-ups):

1. Add a // TODO(#2812) comment at daemon.go:46 noting the floor is daemon-only and ao doctor (CLI process) still needs it — this prevents the next contributor from assuming the wiring is complete.
2. Acknowledge in the PR body or package doc that the login-shell probe is not ported (static floor only), so non-Homebrew/non-default-prefix installs remain unfixed vs the Electron path.

Neither changes runtime behavior; both are documentation. The doctor-process fix and the login-shell probe are legitimate separate PRs.

---

Reviewed by Hermes Agent (AO). Posting as COMMENT per repo review policy — approval reserved for authorized maintainers.

[i-trytoohard]

Re-review — commit cb57e7ab

Thanks @Pritom14 — verified both flagged items against the branch (not just the description):

• ✅ Doctor CLI-process gap — the TODO(#2812 follow-up) at daemon.go is exactly right, including the load-bearing detail that the follow-up fix is calling EnsureFallbackPath() from PersistentPreRunE, not changing the doctor message. This preserves the correct guidance for whoever picks up the follow-up.
• ✅ Login-shell probe scope — package doc in pathfloor.go now explicitly scopes to the static floor and enumerates the uncovered cases (Nix, asdf/mise, custom prefixes). Clear that the daemon remains less PATH-robust than the Electron path for those setups.

Diff is comment-only (zero code changes), so build/test status is unchanged. CI confirms green: build-test, lint, format, api-drift, container, native (macos/ubuntu/windows), scan — all pass. That covers the go build/vet/test I couldn't run locally on first review.

The two minor items I'd marked optional (HookPATH session-side-effect note in the body; a test asserting EnsureFallbackPath() mutates os.Getenv("PATH")) were skipped — fine, both genuinely low-priority and not worth another round.

No remaining issues from me. Core spawn-gate fix is correct, well-scoped, well-tested, and the two scope boundaries are now documented. Ready for an authorized maintainer's sign-off — leaving approval to them per repo policy.

---

Re-reviewed by Hermes Agent (AO).