From 03d053274e7d9ddd8256617e76c6c787f0d0cd5a Mon Sep 17 00:00:00 2001 From: Jeroen Schweitzer Date: Mon, 15 Jun 2026 17:46:11 +0200 Subject: [PATCH] fix(env): one login-shell-derived PATH for every spawned tool (T-439) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Desktop/dock-launched clide inherits a sparse PATH (no ~/.local/bin, brew, nvm, …), so pql/git/claude and PTY tools went missing. T-347 fixed only the toolchain/pql path on Linux; env.dart's expander was still macOS-only and claude/PTY/git used the raw PATH — the breakage recurred per spawn site because there were three divergent expanders. Consolidate into one resolver (lib/src/env/shell_env.dart): - primeLoginShellPath(): probe the user's real login shell once at startup (`$SHELL -l -c`, sentinel-framed, bounded timeout, graceful fallback to the process PATH). Captures the user's actual PATH, not a hardcoded guess. - expandToolPath(): the canonical merge (moved from toolchain_paths, which re-exports it for its tests) — unions the well-known user/local bin dirs. - resolvedToolPath(): currentSearchPath() + expandToolPath, the single call every spawn site uses. Routed through it: PTY children (registry.dart now overrides PATH), git (env.dart → operations.dart), the toolchain probe (toolchain_paths), and hosted claude (agent_bootstrap). Primed in main.dart's !kIsWeb boot. Deleted the macOS-only env.dart copy and the cli_install copy. Tests: new shell_env_test (probe + every fallback + merge); env_test and cli_install_test updated to the consolidated surface. analyze clean, web wasm build still green, make test green. Co-Authored-By: Claude Opus 4.8 (1M context) --- .pql/changelog/ticket_history/2026-06.sql | 1 + .pql/changelog/tickets/2026-06.sql | 20 ++++ CHANGELOG.md | 7 ++ lib/builtin/claude/src/agent_bootstrap.dart | 6 +- lib/kernel/src/cli_install.dart | 17 +--- lib/kernel/src/toolchain_paths.dart | 34 +++---- lib/main.dart | 6 ++ lib/src/env/shell_env.dart | 93 ++++++++++++++++++ lib/src/panes/registry.dart | 4 + lib/src/pty/env.dart | 29 ++---- test/kernel/cli_install_test.dart | 19 +--- test/pty/env_test.dart | 18 ++-- test/src/env/shell_env_test.dart | 102 ++++++++++++++++++++ 13 files changed, 269 insertions(+), 87 deletions(-) create mode 100644 lib/src/env/shell_env.dart create mode 100644 test/src/env/shell_env_test.dart diff --git a/.pql/changelog/ticket_history/2026-06.sql b/.pql/changelog/ticket_history/2026-06.sql index bbc0876a..b966410c 100644 --- a/.pql/changelog/ticket_history/2026-06.sql +++ b/.pql/changelog/ticket_history/2026-06.sql @@ -5454,3 +5454,4 @@ INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, chang **Remaining (why this is `review`, not `done`):** the *compile* gate is in CI, but the full web-WASM **Playwright e2e** (`make test-e2e` / `ui-smoke`: setup-node + `playwright install` in `tools/ui` + serve) is not yet wired into CI — it needs browser/node provisioning on the runner, separate from the fence. `make ui-dev` (build + serve) works now; the Playwright smoke is the follow-on. Acceptance items 1, 3a (compile gate), 4 met; 2/3b (full Playwright e2e) pending that harness wiring.', NULL, '2026-06-15 15:30:48', '2026-06-15 15:30:48', '2026-06-15 15:30:48', NULL, '0cf096a810a7a075307c8df8321b5c33', 2) ON CONFLICT(hash) DO NOTHING; INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FCQ8HB61N3TWVJ8YSMHH2TJ4', 'status', 'in_progress', 'review', NULL, '2026-06-15 15:30:48', '2026-06-15 15:30:48', '2026-06-15 15:30:48', NULL, 'a7d81444607b6ec3aa8440fcbc9abee3', 2) ON CONFLICT(hash) DO NOTHING; INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FCQHWQ40AY6SNVRJ86YWA0J8', 'status', 'backlog', 'in_progress', NULL, '2026-06-15 15:31:15', '2026-06-15 15:31:15', '2026-06-15 15:31:15', NULL, '5fed82be61479c8191ed8d0663507cad', 2) ON CONFLICT(hash) DO NOTHING; +INSERT INTO ticket_history (ticket_record_id, field, old_value, new_value, changed_by, changed_at, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FCQHWQ40AY6SNVRJ86YWA0J8', 'status', 'in_progress', 'in_progress', NULL, '2026-06-15 15:32:16', '2026-06-15 15:32:16', '2026-06-15 15:32:16', NULL, 'c1e48da490b71dea20c50a486600b643', 2) ON CONFLICT(hash) DO NOTHING; diff --git a/.pql/changelog/tickets/2026-06.sql b/.pql/changelog/tickets/2026-06.sql index d8b9ad94..155ea23b 100644 --- a/.pql/changelog/tickets/2026-06.sql +++ b/.pql/changelog/tickets/2026-06.sql @@ -6234,3 +6234,23 @@ So T-347 fixed the *toolchain/pql* path on Linux, but the `env.dart` copy (git, **Acceptance.** Desktop-launched clide on Linux + macOS resolves the same PATH the user''s login shell has; pql/git/claude and PTY children all find user-installed tools; the three divergent expanders are unified into one; graceful fallback when the shell probe fails or times out; covered by a test for the resolver + the fallback. **Related:** T-347 (done — fixed the Linux toolchain/pql path, but only `toolchain_paths.dart`), T-215 (CLIDE_SOCK/CLIDE_WORKSPACE + clide on the child PATH), T-211/T-212 (clide-on-PATH install), D-59 (bundled git) / D-92 (bundled pql) — bundling covers git/pql specifically, but not the general "user''s installed tools" PATH this addresses.', 'in_progress', 'high', NULL, NULL, NULL, '2026-06-15 14:55:15', '2026-06-15 15:31:15', NULL, 'c816196f97e6e80bd828c319f99619c8', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); +INSERT INTO tickets (record_id, type, parent_record_id, title, description, status, priority, assigned_to, team, decision_ref, created_at, updated_at, deleted_at, hash, canonical_version) VALUES ('06FCQHWQ40AY6SNVRJ86YWA0J8', 'bug', NULL, 'Consolidate PATH resolution: one login-shell-derived PATH for every spawned tool (git/pql/claude/PTY)', '**Symptom (user, recurring).** When clide is launched from a desktop/dock launcher (not a terminal), it doesn''t see the normal interactive-shell `PATH`, so spawned tools and installed components go missing — pql, git, claude-invoked CLIs, etc. Launching from a terminal works (the shell PATH is inherited). + +**Root cause.** A GUI/desktop-launched process inherits a minimal `PATH` (roughly `/usr/bin:/bin`) — it never sources `~/.bashrc` / `~/.zprofile` / `/etc/profile.d` / brew shellenv, so `~/.local/bin`, `/opt/homebrew/bin`, and any user-customized dirs (nvm, pyenv, cargo, …) are absent. clide builds the environment for everything it spawns from `Platform.environment`, so that impoverished PATH propagates everywhere. + +**Why it keeps happening — divergent, partial PATH expansion.** There are THREE separate PATH-augmentation implementations, fixed inconsistently: +1. `lib/src/pty/env.dart` → `expandedPath` — **still macOS-only** (`if (!Platform.isMacOS) return base;`). Used by `lib/src/git/operations.dart:24` (git resolution) → on Linux desktop-launch, git gets the raw PATH. +2. `lib/kernel/src/toolchain_paths.dart` → `_expandedPath()` — augments on Linux too (this is what **T-347** fixed, for pql). +3. `lib/kernel/src/cli_install.dart` → `expandedPath(base, {macOS, home})` — a third copy. + +So T-347 fixed the *toolchain/pql* path on Linux, but the `env.dart` copy (git, PTY env defaults) is still macOS-only, and claude''s spawn (`agent_bootstrap.agentEnvDelta` → `Process.start(environment:)` merged over `Platform.environment`) only prepends the clide-CLI dir — the rest of PATH stays un-enriched. Net: the same class of breakage recurs per spawn site because there''s no single source of truth. + +**Also:** all three use a **hardcoded dir list** (`~/.local/bin`, `/opt/homebrew/bin`, `/usr/local/bin`). That misses arbitrary user customizations (nvm/pyenv/cargo/asdf/custom dirs) — the user''s "normal bash PATH" is whatever their login shell actually produces, not a fixed list. + +**Proposed fix (two parts).** +1. *Robust resolution:* derive the real login-shell PATH once at startup — spawn the user''s `$SHELL -l -i -c ''printf %s "$PATH"''` (or `-l -c` to avoid interactive side-effects), with a short timeout and a graceful fallback to the current hardcoded-merge behavior. Cache it for the process. This is the established approach (VS Code''s `resolveShellEnv`, the `fix-path` pattern) and captures the user''s actual PATH, not a guess. +2. *Consolidation:* collapse the three `expandedPath`/`_expandedPath` copies into ONE source of truth (e.g. in `lib/src/pty/env.dart` or a small `kernel` env service) that every spawn site uses — PTY/terminal (`registry.dart:55` currently passes raw `Platform.environment`), git (`operations.dart`), toolchain (`toolchain_paths.dart`), claude (`agent_bootstrap.dart`), and any other `Process.start`. One resolver, applied everywhere. + +**Acceptance.** Desktop-launched clide on Linux + macOS resolves the same PATH the user''s login shell has; pql/git/claude and PTY children all find user-installed tools; the three divergent expanders are unified into one; graceful fallback when the shell probe fails or times out; covered by a test for the resolver + the fallback. + +**Related:** T-347 (done — fixed the Linux toolchain/pql path, but only `toolchain_paths.dart`), T-215 (CLIDE_SOCK/CLIDE_WORKSPACE + clide on the child PATH), T-211/T-212 (clide-on-PATH install), D-59 (bundled git) / D-92 (bundled pql) — bundling covers git/pql specifically, but not the general "user''s installed tools" PATH this addresses.', 'in_progress', 'high', NULL, NULL, NULL, '2026-06-15 14:55:15', '2026-06-15 15:32:16', NULL, 'cd231fbbec9b6faa4659286c24893d30', 2) ON CONFLICT(record_id) DO UPDATE SET type=excluded.type, parent_record_id=excluded.parent_record_id, title=excluded.title, description=excluded.description, status=excluded.status, priority=excluded.priority, assigned_to=excluded.assigned_to, team=excluded.team, decision_ref=excluded.decision_ref, updated_at=excluded.updated_at, deleted_at=excluded.deleted_at, hash=excluded.hash, canonical_version=excluded.canonical_version WHERE excluded.updated_at > tickets.updated_at OR (excluded.updated_at = tickets.updated_at AND excluded.hash > tickets.hash); diff --git a/CHANGELOG.md b/CHANGELOG.md index 30e33d17..b5b59489 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -58,6 +58,13 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit. added to CI so the fence can't silently rot. Desktop builds are unchanged — the web target degrades (no terminal, no native git, no syntax highlighting), it does not compromise desktop fidelity. (T-438, D-100, Q-50) +- **Desktop-launched clide finds your installed tools.** A dock/launcher start + inherits a sparse `PATH` (no `~/.local/bin`, brew, nvm, …), so pql/git/claude + and terminal tools could go missing. clide now resolves the real login-shell + `PATH` once at startup (`$SHELL -l -c`, bounded + graceful fallback) and routes + every spawn site — PTY children, git, the toolchain probe, hosted claude — + through one shared resolver, replacing three divergent (and partly macOS-only) + PATH expanders. (T-439, follows T-347) ## [2.5.0] — 2026-06-14 diff --git a/lib/builtin/claude/src/agent_bootstrap.dart b/lib/builtin/claude/src/agent_bootstrap.dart index 5b4cd61c..01603327 100644 --- a/lib/builtin/claude/src/agent_bootstrap.dart +++ b/lib/builtin/claude/src/agent_bootstrap.dart @@ -23,6 +23,7 @@ library; import 'dart:io'; +import 'package:clide/src/env/shell_env.dart' show resolvedToolPath; import 'package:clide/src/ipc/paths.dart' show workspaceSocketPath; // Web fence (T-438, D-100): `Abi.current()` (dart:ffi) is desktop-only; the web @@ -108,7 +109,10 @@ class AgentBootstrap { /// orchestrator merges both into one `--append-system-prompt`. AgentBootstrap agentBootstrap(String workspaceRoot, {Map? base}) { final home = Platform.environment['HOME']; - final currentPath = (base ?? Platform.environment)['PATH'] ?? Platform.environment['PATH']; + // The login-shell-resolved PATH (T-439) so a hosted claude — and the tools it + // shells out to — find user-installed components on a desktop launch, not just + // the sparse GUI PATH. agentEnvDelta still prepends the clide-CLI dir. + final currentPath = resolvedToolPath(); final candidates = [ if (home != null && home.isNotEmpty) '$home/.local/bin', '$workspaceRoot/native/${currentNativeDirName()}', diff --git a/lib/kernel/src/cli_install.dart b/lib/kernel/src/cli_install.dart index affbd28e..bd09f3ea 100644 --- a/lib/kernel/src/cli_install.dart +++ b/lib/kernel/src/cli_install.dart @@ -14,6 +14,8 @@ library; import 'dart:io'; +import 'package:clide/src/env/shell_env.dart' show expandToolPath; + /// State of the `clide` shell command relative to the running GUI. enum CliInstallState { /// No `clide` resolves on PATH. @@ -209,7 +211,7 @@ class CliInstaller { static String get _pathSep => Platform.isWindows ? ';' : ':'; - String _expandedPath() => expandedPath(env['PATH'] ?? '', macOS: Platform.isMacOS, home: env['HOME'] ?? ''); + String _expandedPath() => expandToolPath(env['PATH'] ?? '', isMac: Platform.isMacOS, isLinux: Platform.isLinux, home: env['HOME'] ?? ''); /// `~/.local/bin` on every platform — on Windows that is /// `%USERPROFILE%\.local\bin`, the same convention the claude and @@ -234,16 +236,3 @@ final RegExp _devTreeClient = RegExp(r'(^|/)native/(linux|macos|windows)-(x64|ar /// PATH; it's a working client but not a packaged production install, so it's /// classified separately (T-256) rather than as a clean install. bool isDevTreeClient(String path) => _devTreeClient.hasMatch(path.replaceAll('\\', '/')); - -/// Expand a `PATH` value. Mirrors `toolchain_paths.dart`: macOS GUI apps -/// launch with a sparse PATH that omits the usual user/homebrew bins, so on -/// macOS we prepend those (de-duplicated) before scanning. A top-level, -/// platform-parameterized function so both branches are testable off-platform. -String expandedPath(String base, {required bool macOS, String home = ''}) { - if (!macOS) return base; - final extras = [if (home.isNotEmpty) '$home/.local/bin', '/opt/homebrew/bin', '/opt/homebrew/sbin', '/usr/local/bin']; - final existing = base.split(':').toSet(); - final missing = extras.where((p) => !existing.contains(p)); - if (missing.isEmpty) return base; - return [...missing, ...existing].join(':'); -} diff --git a/lib/kernel/src/toolchain_paths.dart b/lib/kernel/src/toolchain_paths.dart index b6d42027..7df1bfb6 100644 --- a/lib/kernel/src/toolchain_paths.dart +++ b/lib/kernel/src/toolchain_paths.dart @@ -10,6 +10,13 @@ library; import 'dart:io'; +import 'package:clide/src/env/shell_env.dart'; + +// The canonical PATH-expansion logic now lives in shell_env (T-439, the single +// source of truth shared with git/pql/PTY/claude). Re-exported so existing +// importers/tests keep resolving it from here. +export 'package:clide/src/env/shell_env.dart' show expandToolPath; + /// Serializable result of tool resolution (crosses isolate boundary). class ResolvedPaths { const ResolvedPaths({this.git, this.pql, this.shell, this.gitEnv}); @@ -144,25 +151,8 @@ String? _firstExisting(List candidates) { return null; } -/// Build expanded PATH inline — must be self-contained for isolate use. -String _expandedPath() => - expandToolPath(Platform.environment['PATH'] ?? '', isMac: Platform.isMacOS, isLinux: Platform.isLinux, home: Platform.environment['HOME']); - -/// Pure PATH-expansion logic, extracted so it's testable without touching the -/// process environment. -/// -/// A desktop-launched app (macOS or Linux) inherits a minimal PATH that lacks -/// the user bin dirs where tools like `pql` install (`~/.local/bin`), so tool -/// resolution fails even though a terminal launch would find them. Re-add the -/// common user/local bin dirs — that any are missing means they're prepended, -/// so they take precedence over a stale system copy (T-347). Homebrew dirs are -/// macOS-only. On other platforms the base PATH passes through unchanged. -String expandToolPath(String base, {required bool isMac, required bool isLinux, String? home}) { - if (!isMac && !isLinux) return base; - final h = home ?? ''; - final extras = [if (h.isNotEmpty) '$h/.local/bin', if (isMac) '/opt/homebrew/bin', if (isMac) '/opt/homebrew/sbin', '/usr/local/bin']; - final existing = base.split(':').toSet(); - final missing = extras.where((p) => !existing.contains(p)); - if (missing.isEmpty) return base; - return [...missing, ...existing].join(':'); -} +/// The full tool search PATH — the login-shell PATH (probed once at startup) +/// unioned with the well-known user/local bin dirs, shared with every other +/// spawn site via [shell_env] (T-439). In an isolate that never primed the +/// probe it degrades to the process PATH + the well-known dirs (T-347). +String _expandedPath() => resolvedToolPath(); diff --git a/lib/main.dart b/lib/main.dart index f606be32..b93e92d7 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -51,6 +51,7 @@ import 'package:clide/src/daemon/search_commands.dart'; import 'package:clide/src/editor/registry.dart' show EditorRegistry; import 'package:clide/src/git/client.dart'; import 'package:clide/src/cli/argv_dispatch.dart'; +import 'package:clide/src/env/shell_env.dart' show primeLoginShellPath; import 'package:clide/src/ipc/envelope.dart'; import 'package:clide/src/ipc/mcp_server.dart'; import 'package:clide/src/ipc/paths.dart' show workspaceSocketPath, logDirectory; @@ -100,6 +101,11 @@ Future main() async { LogLevel bootLogLevel = kReleaseMode ? LogLevel.warn : LogLevel.info; List bootLogSinks = const []; if (!kIsWeb) { + // Resolve the user's real login-shell PATH once, before any tool resolution + // or spawn — a desktop/dock launch inherits a sparse PATH that misses + // ~/.local/bin, brew, nvm, etc. (T-439). Bounded + graceful: a slow/failed + // probe just falls back to the process PATH + well-known dirs. + await primeLoginShellPath(); final bootSettings = SettingsStore(appDir: appDir); await bootSettings.load(); startupWorkRoot = resolveStartupWorkspace( diff --git a/lib/src/env/shell_env.dart b/lib/src/env/shell_env.dart new file mode 100644 index 00000000..9d47fd82 --- /dev/null +++ b/lib/src/env/shell_env.dart @@ -0,0 +1,93 @@ +/// The single source of truth for the PATH clide hands to every tool it +/// spawns — git, pql, the toolchain probe, PTY children, hosted claude (T-439). +/// +/// A desktop/dock-launched GUI process inherits a minimal PATH (roughly +/// `/usr/bin:/bin`): it never sources `~/.bashrc` / `~/.zprofile` / +/// `/etc/profile.d` / brew shellenv, so `~/.local/bin`, Homebrew, and any +/// user-customized dirs (nvm/pyenv/cargo/asdf/…) are absent and tool resolution +/// fails even though a terminal launch would find them. Two layers, in order: +/// +/// 1. [primeLoginShellPath] probes the user's actual login shell once at +/// startup (`$SHELL -l -c …`) — the real PATH, not a guess — and caches it. +/// 2. [expandToolPath] additionally unions in the well-known user/local bin +/// dirs, so resolution still works when the probe is unavailable (Windows, +/// timeout, spawn failure) or the shell's profile omits a dir we know. +/// +/// Flutter-free (used by `GitClient` / `PqlClient` under `dart test`). +library; + +import 'dart:io'; + +String? _loginShellPath; +bool _primed = false; + +/// Probe the user's login shell for its `PATH`, once, and cache it. Desktop-only +/// — the caller guards on `!kIsWeb`. Idempotent. Graceful: on Windows (no +/// login-shell convention), a missing `$SHELL`, a non-zero exit, a timeout, or a +/// spawn failure, the cache stays null and [currentSearchPath] falls back to the +/// process `PATH` (still hardcoded-merged by [expandToolPath]). +/// +/// [run] is injectable for tests; [timeout] bounds the probe so a misbehaving +/// profile can never hang startup. +Future primeLoginShellPath({ + Future Function(String executable, List arguments)? run, + String? shell, + Duration timeout = const Duration(seconds: 4), +}) async { + if (_primed) return; + _primed = true; + if (Platform.isWindows) return; // PowerShell has no `-l -c` PATH convention. + final sh = shell ?? Platform.environment['SHELL']; + if (sh == null || sh.isEmpty) return; + final runner = run ?? (e, a) => Process.run(e, a); + try { + // `-l -c`: a login shell (sources the profile files that set the real PATH) + // but non-interactive (no prompt, no hang). Frame the value in sentinels so + // any MOTD / profile chatter on stdout is stripped. + final res = await runner(sh, ['-l', '-c', r'printf "__CLIDE_PATH__%s__CLIDE_PATH__" "$PATH"']).timeout(timeout); + if (res.exitCode != 0) return; + final out = res.stdout is String ? res.stdout as String : ''; + final m = RegExp(r'__CLIDE_PATH__(.*?)__CLIDE_PATH__', dotAll: true).firstMatch(out); + final path = m?.group(1)?.trim(); + if (path != null && path.isNotEmpty) _loginShellPath = path; + } catch (_) { + // timeout / spawn failure → leave the cache null and fall back. + } +} + +/// The base search PATH: the login-shell PATH if [primeLoginShellPath] resolved +/// one, else the process `PATH`. Not yet merged with the well-known dirs — use +/// [resolvedToolPath] for the full search path. +String currentSearchPath() => _loginShellPath ?? Platform.environment['PATH'] ?? ''; + +/// The full PATH clide should hand to spawned tools: the login-shell/process +/// PATH unioned with the well-known user/local bin dirs. The single resolver +/// every spawn site calls. +String resolvedToolPath() => expandToolPath(currentSearchPath(), isMac: Platform.isMacOS, isLinux: Platform.isLinux, home: Platform.environment['HOME']); + +/// Pure PATH-expansion: prepend the well-known user/local bin dirs that a +/// desktop launch drops, de-duplicated, so they take precedence over a stale +/// system copy (T-347). Homebrew dirs are macOS-only. On platforms that aren't +/// macOS/Linux the base passes through unchanged. Extracted so it's testable +/// without touching the process environment. +String expandToolPath(String base, {required bool isMac, required bool isLinux, String? home}) { + if (!isMac && !isLinux) return base; + final h = home ?? ''; + final extras = [if (h.isNotEmpty) '$h/.local/bin', if (isMac) '/opt/homebrew/bin', if (isMac) '/opt/homebrew/sbin', '/usr/local/bin']; + final existing = base.split(':').toSet(); + final missing = extras.where((p) => !existing.contains(p)); + if (missing.isEmpty) return base; + return [...missing, ...existing].join(':'); +} + +/// Test seam: force the cached login-shell PATH (and mark primed). +void debugSetLoginShellPath(String? value) { + _loginShellPath = value; + _primed = true; +} + +/// Test seam: clear the cache so [primeLoginShellPath] probes again. +void debugResetLoginShellPath() { + _loginShellPath = null; + _primed = false; +} diff --git a/lib/src/panes/registry.dart b/lib/src/panes/registry.dart index c8f4a10e..f4b015c6 100644 --- a/lib/src/panes/registry.dart +++ b/lib/src/panes/registry.dart @@ -11,6 +11,7 @@ import 'dart:convert'; import 'dart:io' show Platform; import 'dart:typed_data'; +import '../env/shell_env.dart' show resolvedToolPath; import '../ipc/envelope.dart'; import '../pty/pty_log.dart'; import '../pty/pty_session.dart'; @@ -53,6 +54,9 @@ class PaneRegistry { // Terminal defaults for the PTY child. final fullEnv = { ...Platform.environment, + // The login-shell-resolved PATH so PTY children find user-installed tools + // even on a desktop launch (T-439); an explicit caller PATH still wins. + 'PATH': resolvedToolPath(), 'TERM': 'xterm-256color', 'COLORTERM': 'truecolor', 'LANG': 'en_US.UTF-8', diff --git a/lib/src/pty/env.dart b/lib/src/pty/env.dart index f98dd5f4..1b74bd50 100644 --- a/lib/src/pty/env.dart +++ b/lib/src/pty/env.dart @@ -8,29 +8,14 @@ /// the renderer can do true colour. library; -import 'dart:io'; +import 'package:clide/src/env/shell_env.dart' show resolvedToolPath; -/// On macOS, GUI apps inherit a minimal PATH that omits Homebrew, -/// ~/.local/bin, and similar directories. This getter returns the -/// platform PATH with those well-known directories merged in. -/// On Linux/Windows it returns the PATH unchanged. -String get expandedPath { - _cachedPath ??= _buildExpandedPath(); - return _cachedPath!; -} - -String? _cachedPath; - -String _buildExpandedPath() { - final base = Platform.environment['PATH'] ?? ''; - if (!Platform.isMacOS) return base; - final home = Platform.environment['HOME'] ?? ''; - final extras = [if (home.isNotEmpty) '$home/.local/bin', '/opt/homebrew/bin', '/opt/homebrew/sbin', '/usr/local/bin']; - final existing = base.split(':').toSet(); - final missing = extras.where((p) => !existing.contains(p)); - if (missing.isEmpty) return base; - return [...missing, ...existing].join(':'); -} +/// The full tool search PATH for PTY children and PATH-resolved subprocess +/// lookup — delegates to the shared resolver ([resolvedToolPath], T-439) so +/// every spawn site agrees: the login-shell PATH (probed once at startup) +/// unioned with the well-known user/local bin dirs. Previously this was a +/// macOS-only merge, so a Linux desktop launch left tools unresolvable. +String get expandedPath => resolvedToolPath(); /// Base env clide builds for every PTY child. Callers merge with the /// user's environment — a child that needs user env like `HOME` / diff --git a/test/kernel/cli_install_test.dart b/test/kernel/cli_install_test.dart index 87a25c40..c8b19c69 100644 --- a/test/kernel/cli_install_test.dart +++ b/test/kernel/cli_install_test.dart @@ -130,23 +130,8 @@ void main() { }); }); - group('expandedPath', () { - test('non-macOS returns PATH unchanged', () { - expect(expandedPath('/a:/b', macOS: false, home: '/home/x'), '/a:/b'); - }); - - test('macOS prepends missing user + homebrew bins', () { - final out = expandedPath('/usr/bin', macOS: true, home: '/home/x').split(':'); - expect(out, contains('/home/x/.local/bin')); - expect(out, contains('/opt/homebrew/bin')); - expect(out.last, '/usr/bin'); - }); - - test('macOS does not duplicate entries already on PATH', () { - final out = expandedPath('/opt/homebrew/bin:/usr/bin', macOS: true, home: ''); - expect('/opt/homebrew/bin'.allMatches(out).length, 1); - }); - }); + // PATH expansion moved to the shared resolver (T-439); its logic is covered by + // expandToolPath in toolchain_paths_test + shell_env_test. group('install', () { test('fails clearly when no bundled client is present', () { diff --git a/test/pty/env_test.dart b/test/pty/env_test.dart index 8beb8a5b..cd0da35b 100644 --- a/test/pty/env_test.dart +++ b/test/pty/env_test.dart @@ -23,20 +23,16 @@ void main() { }); }); - group('expandedPath', () { - test('returns a non-empty string on every platform', () { + group('expandedPath (delegates to the shared resolver, T-439)', () { + test('non-empty on every platform', () { expect(expandedPath, isNotEmpty); }); - test('on Linux/Windows, equals Platform.environment[PATH]', () { - if (Platform.isMacOS) return; // macOS path-merge tested separately. - expect(expandedPath, Platform.environment['PATH']); - }); - - test('on macOS, includes the well-known extras', () { - if (!Platform.isMacOS) return; - // Homebrew is the canonical one; at least one of these should appear. - expect(expandedPath, anyOf(contains('/opt/homebrew/bin'), contains('/usr/local/bin'))); + test('is a superset of the process PATH (never drops entries)', () { + final got = expandedPath.split(':').toSet(); + for (final dir in (Platform.environment['PATH'] ?? '').split(':').where((e) => e.isNotEmpty)) { + expect(got, contains(dir)); + } }); }); diff --git a/test/src/env/shell_env_test.dart b/test/src/env/shell_env_test.dart new file mode 100644 index 00000000..bfe941b3 --- /dev/null +++ b/test/src/env/shell_env_test.dart @@ -0,0 +1,102 @@ +/// Tests for the consolidated PATH resolver (T-439): the login-shell probe, +/// its graceful fallbacks, and the shared `expandToolPath` merge. +library; + +import 'dart:io'; + +import 'package:clide/src/env/shell_env.dart'; +import 'package:test/test.dart'; + +ProcessResult _ok(String path) => ProcessResult(1, 0, '__CLIDE_PATH__${path}__CLIDE_PATH__', ''); + +void main() { + setUp(debugResetLoginShellPath); + tearDown(debugResetLoginShellPath); + + group('primeLoginShellPath', () { + test('caches the login shell PATH so currentSearchPath returns it', () async { + await primeLoginShellPath(shell: '/bin/zsh', run: (e, a) async => _ok('/opt/tool/bin:/usr/bin')); + expect(currentSearchPath(), '/opt/tool/bin:/usr/bin'); + }); + + test('strips profile chatter around the sentinel-framed PATH', () async { + await primeLoginShellPath(shell: '/bin/bash', run: (e, a) async => ProcessResult(1, 0, 'MOTD: hi\n__CLIDE_PATH__/a:/b__CLIDE_PATH__', '')); + expect(currentSearchPath(), '/a:/b'); + }); + + test('falls back to the process PATH on a non-zero exit', () async { + await primeLoginShellPath(shell: '/bin/bash', run: (e, a) async => ProcessResult(1, 1, '', 'boom')); + expect(currentSearchPath(), Platform.environment['PATH'] ?? ''); + }); + + test('falls back when the probe throws (e.g. spawn failure)', () async { + await primeLoginShellPath(shell: '/bin/bash', run: (e, a) async => throw const ProcessException('sh', [])); + expect(currentSearchPath(), Platform.environment['PATH'] ?? ''); + }); + + test('falls back when the probe times out', () async { + await primeLoginShellPath( + shell: '/bin/bash', + timeout: const Duration(milliseconds: 20), + run: (e, a) => Future.delayed(const Duration(seconds: 5), () => _ok('/never')), + ); + expect(currentSearchPath(), Platform.environment['PATH'] ?? ''); + }); + + test('falls back when SHELL is empty', () async { + await primeLoginShellPath(shell: '', run: (e, a) async => _ok('/should/not/run')); + expect(currentSearchPath(), Platform.environment['PATH'] ?? ''); + }); + + test('is idempotent — a second call does not re-probe', () async { + var calls = 0; + await primeLoginShellPath( + shell: '/bin/bash', + run: (e, a) async { + calls++; + return _ok('/first'); + }, + ); + await primeLoginShellPath( + shell: '/bin/bash', + run: (e, a) async { + calls++; + return _ok('/second'); + }, + ); + expect(calls, 1); + expect(currentSearchPath(), '/first'); + }); + }); + + group('resolvedToolPath', () { + test('unions the well-known dirs onto the resolved base', () async { + await primeLoginShellPath(shell: '/bin/bash', run: (e, a) async => _ok('/usr/bin')); + final got = resolvedToolPath(); + // The resolved base is preserved; on macOS/Linux the user/local dirs are + // unioned in. (Windows passes through, so only assert the base is kept.) + expect(got.split(':'), contains('/usr/bin')); + if (Platform.isLinux || Platform.isMacOS) { + expect(got.split(':'), contains('/usr/local/bin')); + } + }); + }); + + group('expandToolPath', () { + test('prepends missing user/local dirs (Linux), de-duplicated, base last', () { + final out = expandToolPath('/usr/bin', isMac: false, isLinux: true, home: '/home/u').split(':'); + expect(out, contains('/home/u/.local/bin')); + expect(out, contains('/usr/local/bin')); + expect(out.last, '/usr/bin'); + }); + + test('does not duplicate dirs already present', () { + final out = expandToolPath('/usr/local/bin:/usr/bin', isMac: false, isLinux: true, home: ''); + expect('/usr/local/bin'.allMatches(out).length, 1); + }); + + test('passes the base through unchanged off macOS/Linux', () { + expect(expandToolPath('/a:/b', isMac: false, isLinux: false, home: '/home/u'), '/a:/b'); + }); + }); +}