diff --git a/CHANGELOG.md b/CHANGELOG.md index bfad8254..23f967fb 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,6 +26,11 @@ heading, and (b) bumping `pubspec.yaml` `version:` in the same commit. context panel — notes are nodes, wikilinks edges. Hover highlights a note's neighbourhood; click opens it. Scroll to zoom, drag to pan; filter by path glob, tag include/exclude, or depth from the active note. (T-323) +- **Per-workspace PATH preset.** Directories prepended to the PATH of every + shell clide spawns for a repo — Claude sessions and terminal panes. Settings + → Tools → Workspace PATH, or `clide env path …` with capture-from-login-shell + suggestions. Worktrees share their repo's preset; stored machine-local, never + committed. (D-106, T-511) ### Changed diff --git a/lib/builtin/claude/src/agent_bootstrap.dart b/lib/builtin/claude/src/agent_bootstrap.dart index 91941af1..78713593 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/path_preset.dart' show applyPathPreset; import 'package:clide/src/env/shell_env.dart' show resolvedToolPath; import 'package:clide/src/ipc/paths.dart' show workspaceSocketPath; @@ -74,12 +75,22 @@ String clideSkillsNote() => /// /// * `CLIDE_SOCK` — the per-workspace socket ([workspaceSocketPath], D-70). /// * `CLIDE_WORKSPACE` — the workspace root. -/// * `PATH` — prepended with [clideCliDir] when it is non-null (i.e. `clide` -/// is not already resolvable), otherwise left untouched. -Map agentEnvDelta({required String workspaceRoot, required String socketPath, required String? currentPath, required String? clideCliDir}) { +/// * `PATH` — the workspace's preset dirs (D-106), then [clideCliDir] when it +/// is non-null (i.e. `clide` is not already resolvable), then +/// [currentPath]. Exported whenever there is anything to prepend — +/// the preset must reach the session's Bash tool even when `clide` is +/// already on PATH — and left untouched otherwise. +Map agentEnvDelta({ + required String workspaceRoot, + required String socketPath, + required String? currentPath, + required String? clideCliDir, + List prependDirs = const [], +}) { final delta = {'CLIDE_SOCK': socketPath, 'CLIDE_WORKSPACE': workspaceRoot}; - if (clideCliDir != null && clideCliDir.isNotEmpty) { - delta['PATH'] = (currentPath == null || currentPath.isEmpty) ? clideCliDir : '$clideCliDir:$currentPath'; + final prepend = [...prependDirs, if (clideCliDir != null && clideCliDir.isNotEmpty) clideCliDir]; + if (prepend.isNotEmpty) { + delta['PATH'] = applyPathPreset(currentPath ?? '', prepend); } return delta; } @@ -132,11 +143,18 @@ class AgentBootstrap { /// env (usually null → inherit clide's). The returned [AgentBootstrap.extraArgs] /// carries the context note; team callers append their own preamble and the /// orchestrator merges both into one `--append-system-prompt`. -AgentBootstrap agentBootstrap(String workspaceRoot, {Map? base, String? Function(String cwd)? boundConfigDir}) { +AgentBootstrap agentBootstrap( + String workspaceRoot, { + Map? base, + String? Function(String cwd)? boundConfigDir, + List Function(String cwd)? pathPreset, +}) { final home = Platform.environment['HOME']; // 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. + // the sparse GUI PATH. agentEnvDelta still prepends the clide-CLI dir and the + // workspace's PATH preset (D-106), injected as a plain lookup like + // [boundConfigDir] so this stays Flutter-free. final currentPath = resolvedToolPath(); final candidates = [ if (home != null && home.isNotEmpty) '$home/.local/bin', @@ -144,7 +162,13 @@ AgentBootstrap agentBootstrap(String workspaceRoot, {Map? base, File(Platform.resolvedExecutable).parent.path, ]; final cliDir = resolveClideCliDir(currentPath: currentPath, candidateDirs: candidates, isExecutableFile: _isExecutableFile); - final delta = agentEnvDelta(workspaceRoot: workspaceRoot, socketPath: workspaceSocketPath(workspaceRoot), currentPath: currentPath, clideCliDir: cliDir); + final delta = agentEnvDelta( + workspaceRoot: workspaceRoot, + socketPath: workspaceSocketPath(workspaceRoot), + currentPath: currentPath, + clideCliDir: cliDir, + prependDirs: pathPreset?.call(workspaceRoot) ?? const [], + ); // Per-repo Claude account (T-484): a bound workspace runs claude under that // account's CLAUDE_CONFIG_DIR. Spread BEFORE base so an explicit per-call // SpawnSpec.env override still wins (precedence: override > binding > parent diff --git a/lib/builtin/claude/src/extension.dart b/lib/builtin/claude/src/extension.dart index 8cb5283e..ed4a7219 100644 --- a/lib/builtin/claude/src/extension.dart +++ b/lib/builtin/claude/src/extension.dart @@ -3,6 +3,7 @@ import 'dart:io'; import 'package:clide/clide.dart'; import 'package:clide/builtin/claude/src/account_registry.dart'; +import 'package:clide/src/env/path_preset.dart' show presetDirsFrom; import 'package:clide/builtin/claude/src/account_login_dialog.dart'; import 'package:clide/builtin/claude/src/account_roadblock_dialog.dart'; import 'package:clide/builtin/claude/src/account_settings_control.dart'; @@ -511,8 +512,13 @@ class ClaudeExtension extends ClideExtension { // The clide-managed session set (T-169). Panes spawn/bind through it so a // session outlives its pane and is shared across surfaces. The account - // registry (T-476) lets a bound workspace spawn under its own Claude account. - _orchestrator = ClaudeSessionOrchestrator(accountRegistry: AccountRegistry(ctx.settings)); + // registry (T-476) lets a bound workspace spawn under its own Claude + // account; the PATH-preset lookup (D-106) prepends the workspace's preset + // dirs to every hosted session's PATH — read live at each spawn. + _orchestrator = ClaudeSessionOrchestrator( + accountRegistry: AccountRegistry(ctx.settings), + pathPresetFor: (cwd) => presetDirsFrom((k) => ctx.settings.get(k), cwd), + ); activeSessionOrchestrator = _orchestrator; // An in-place workspace switch (Open Project/Folder) must not leave the diff --git a/lib/builtin/claude/src/session_orchestrator.dart b/lib/builtin/claude/src/session_orchestrator.dart index 55587a4d..afe274ae 100644 --- a/lib/builtin/claude/src/session_orchestrator.dart +++ b/lib/builtin/claude/src/session_orchestrator.dart @@ -153,7 +153,7 @@ class ManagedSession { ClaudeSessionOrchestrator? activeSessionOrchestrator; class ClaudeSessionOrchestrator extends ChangeNotifier { - ClaudeSessionOrchestrator({ProcessFactory? processFactory, this.accountRegistry}) : _factory = processFactory ?? _spawnClaude { + ClaudeSessionOrchestrator({ProcessFactory? processFactory, this.accountRegistry, this.pathPresetFor}) : _factory = processFactory ?? _spawnClaude { _chatModel = TeamChatModel(broker: broker, sessionResolver: (name) => byMemberName(name)?.session); } @@ -162,6 +162,11 @@ class ClaudeSessionOrchestrator extends ChangeNotifier { /// Null in tests / when no registry is wired → no injection. final AccountRegistry? accountRegistry; + /// Per-workspace PATH preset lookup (D-106): dirs prepended to a hosted + /// session's PATH at spawn, wired by the extension over the settings store. + /// Null in tests / when not wired → no injection. + final List Function(String cwd)? pathPresetFor; + final ProcessFactory _factory; final _sessions = {}; @@ -255,7 +260,12 @@ class ClaudeSessionOrchestrator extends ChangeNotifier { mcpServers.add(TeamMcpServer(broker: broker, memberId: spec.id)); preambles.add(_teamSystemPrompt(name, spec.role)); } - final bootstrap = agentBootstrap(spec.cwd, base: spec.env, boundConfigDir: (cwd) => accountRegistry?.accountForWorkspace(cwd)?.dir); + final bootstrap = agentBootstrap( + spec.cwd, + base: spec.env, + boundConfigDir: (cwd) => accountRegistry?.accountForWorkspace(cwd)?.dir, + pathPreset: pathPresetFor, + ); sessionArgs = [ '--append-system-prompt', preambles.join('\n\n'), diff --git a/lib/main.dart b/lib/main.dart index 81f05c05..f6ea412b 100644 --- a/lib/main.dart +++ b/lib/main.dart @@ -40,6 +40,7 @@ import 'package:clide/builtin/claude/src/account_registry.dart'; import 'package:clide/clide.dart' show clideVersion; import 'package:clide/src/daemon/claude_account_commands.dart'; import 'package:clide/src/daemon/dispatcher.dart'; +import 'package:clide/src/daemon/env_path_commands.dart'; import 'package:clide/src/daemon/draw_commands.dart'; import 'package:clide/src/draw/compare_template.dart' show compareTemplateHandler; import 'package:clide/src/draw/d2_template.dart' show d2TemplateHandler; @@ -62,7 +63,8 @@ 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/env/path_preset.dart' show applyPathPreset, pathPresetKey, presetDirsFrom; +import 'package:clide/src/env/shell_env.dart' show primeLoginShellPath, resolvedToolPath; import 'package:clide/src/env/supporter_binaries.dart'; import 'package:clide/widgets/src/icons/phosphor_glyphs.g.dart' show kPhosphorGlyphs; import 'package:clide/src/ipc/envelope.dart'; @@ -310,7 +312,19 @@ Future main() async { crumbPath: '${logDirectory()}/clide-pty.crumbs.log', verbose: log.minLevel.index <= LogLevel.debug.index, ); - final paneRegistry = PaneRegistry(events: eventSink, ptyLog: ptyLog); + // Terminal PTY children get the workspace's PATH preset (D-106) prepended + // to the resolved login-shell PATH. Read live per spawn off the kernel + // settings (null pre-boot → plain resolved PATH, same as before). + final paneRegistry = PaneRegistry( + events: eventSink, + ptyLog: ptyLog, + pathForSpawn: (cwd) { + final settings = kernelSettings; + final base = resolvedToolPath(); + if (settings == null) return base; + return applyPathPreset(base, presetDirsFrom((k) => settings.get(k), cwd ?? workRoot.path)); + }, + ); // D-6 parity (T-219, D-83): make the tabs the user sees in the GUI // visible to `pane list` by snapshotting the kernel PanelRegistry + // LayoutArrangement at request time — no mirrored state to drift. @@ -440,6 +454,18 @@ Future main() async { publisher: () => kernelMessages?.publish, workspaceCwd: () => workRoot.path, ); + // `clide env path …` — the per-workspace PATH preset's CLI half (D-106, + // T-511). Reads/writes the user-scope preset key through the kernel + // settings; mutations publish on envPathChannel. + registerEnvPathCommands( + dispatcher, + () { + final settings = kernelSettings; + return settings == null ? null : _PathPresetStoreAdapter(settings); + }, + publisher: () => kernelMessages?.publish, + workspaceCwd: () => workRoot.path, + ); // `clide status` — one-shot orientation snapshot (T-221): active pane, // focused file + selection, git summary, layout. Assembled here where the // live kernel + subsystem state is in scope; the reader's viewed doc is @@ -655,6 +681,24 @@ class _AccountStoreAdapter implements AccountStore { Future unbind(String cwd) => _reg.unbindWorkspace(cwd); } +/// Adapts the (foundation-bound) [SettingsStore] to the Flutter-free +/// [PathPresetStore] port the `env path` verbs use (D-106, T-511). The key is +/// worktree-aware ([pathPresetKey]); an empty preset removes the key rather +/// than leaving an empty list in settings.yaml. +class _PathPresetStoreAdapter implements PathPresetStore { + _PathPresetStoreAdapter(this._settings); + final SettingsStore _settings; + + @override + List dirsFor(String cwd) => presetDirsFrom((k) => _settings.get(k), cwd); + + @override + Future setFor(String cwd, List dirs) { + final key = pathPresetKey(cwd); + return dirs.isEmpty ? _settings.removeAt(SettingsScope.app, key) : _settings.setAt(SettingsScope.app, key, dirs); + } +} + class _BusEventSink implements DaemonEventSink { _BusEventSink(this._bus); final DaemonBus _bus; diff --git a/lib/src/daemon/env_path_commands.dart b/lib/src/daemon/env_path_commands.dart new file mode 100644 index 00000000..6e5ba1b2 --- /dev/null +++ b/lib/src/daemon/env_path_commands.dart @@ -0,0 +1,203 @@ +/// Registers the `env path …` verbs — the CLI half of the per-workspace PATH +/// preset (D-106, T-511; D-6 CLI parity with the Tools settings section). +/// +/// `clide env path list` +/// `clide env path set [ …]` +/// `clide env path add [ …]` +/// `clide env path remove [ …]` +/// `clide env path clear` +/// `clide env path capture` +/// +/// The argv grammar splits the first two tokens as `subsystem.verb`, so the +/// command id is `env.path` and the sub-verb arrives as the first positional; +/// the trailing `dirs` positional is variadic. Preset reads/writes go through +/// an injected [PathPresetStore] port and mutations are published on +/// [envPathChannel] — keeping this handler Flutter-free so it runs under +/// `dart test`. +/// +/// Preset dirs deliberately live OUTSIDE the workspace (`~/.nvm/…`, linuxbrew), +/// so there is no workspace-confinement check here — the guards are absolute +/// paths only (after `~/` expansion) and the T-104 leading-dash rejection. +/// Nonexistent dirs warn (in the `missing` field), never error: a preset may +/// predate the dir it names. +library; + +import 'dart:io' show Directory, Platform; + +import '../env/path_preset.dart'; +import '../env/shell_env.dart' show loginShellPathOrNull, resolvedToolPath; +import '../ipc/command_schema.dart'; +import '../ipc/envelope.dart'; +import '../ipc/schema_v1.dart'; +import 'dispatcher.dart'; +import 'ui_command.dart' show MessagePublisher; + +/// The MessageBus channel preset mutations publish on; UI surfaces subscribe +/// to reflect a CLI edit (the settings control also re-reads off the store +/// notifier, so this is the event-contract half of D-6). +const envPathChannel = 'env.path'; + +/// Flutter-free port over the (foundation-bound) SettingsStore, injected so +/// this command runs under `dart test`. main.dart adapts the real store to it. +abstract class PathPresetStore { + /// The preset for the workspace at [cwd], resolved to its repo identity + /// ([presetRootFor]) — a worktree reads its main repo's preset. + List dirsFor(String cwd); + + /// Replace the preset for [cwd]'s repo. An empty [dirs] removes the key. + Future setFor(String cwd, List dirs); +} + +/// Register `env.path`. [store] / [publisher] / [workspaceCwd] are late-bound +/// closures (captured post-boot in main.dart); each may be null in a headless +/// context, in which case the verb degrades to a clear error. [home] / +/// [dirExists] / [loginPath] / [processPath] are test seams over the real +/// environment. +void registerEnvPathCommands( + DaemonDispatcher d, + PathPresetStore? Function() store, { + MessagePublisher? Function()? publisher, + String? Function()? workspaceCwd, + String? Function()? home, + bool Function(String dir)? dirExists, + String? Function()? loginPath, + String Function()? processPath, +}) { + d.register( + 'env.path', + (req) async => _dispatch( + req, + store(), + publisher?.call(), + workspaceCwd?.call(), + home: home ?? () => Platform.environment['HOME'], + dirExists: dirExists ?? (dir) => Directory(dir).existsSync(), + loginPath: loginPath ?? loginShellPathOrNull, + processPath: processPath ?? () => Platform.environment['PATH'] ?? '', + ), + schema: const CommandSchema( + positional: ['action', 'dirs'], + args: { + 'action': ArgSpec(required: true, rejectLeadingDash: true), + 'dirs': ArgSpec(type: ArgType.stringList, rejectLeadingDash: true, maxItems: 64), + }, + ), + ); +} + +Future _dispatch( + IpcRequest req, + PathPresetStore? store, + MessagePublisher? publish, + String? cwd, { + required String? Function() home, + required bool Function(String dir) dirExists, + required String? Function() loginPath, + required String Function() processPath, +}) async { + if (store == null) return _err(req.id, 'settings unavailable in this context'); + if (cwd == null) return _err(req.id, 'no workspace open'); + final action = (req.args['action'] as String?)?.trim(); + final rawDirs = (req.args['dirs'] as List?)?.cast() ?? const []; + final root = presetRootFor(cwd); + + Future mutate(List dirs) async { + await store.setFor(cwd, dirs); + publish?.call('cli', envPathChannel, {'action': action, 'root': root, 'dirs': dirs}); + return _ok(req.id, {'root': root, 'dirs': dirs, 'missing': _missing(dirs, dirExists)}); + } + + switch (action) { + case 'list': + final dirs = store.dirsFor(cwd); + return _ok(req.id, {'root': root, 'dirs': dirs, 'missing': _missing(dirs, dirExists), 'effectivePath': applyPathPreset(resolvedToolPath(), dirs)}); + + case 'set': + if (rawDirs.isEmpty) return _err(req.id, 'set requires at least one ', hint: 'to empty the preset use: clide env path clear'); + final (dirs, bad) = _expandAll(rawDirs, home()); + if (bad != null) return _err(req.id, bad, hint: 'preset entries must be absolute paths (or ~/…)'); + return mutate(_dedupe(dirs)); + + case 'add': + if (rawDirs.isEmpty) return _err(req.id, 'add requires at least one '); + final (dirs, bad) = _expandAll(rawDirs, home()); + if (bad != null) return _err(req.id, bad, hint: 'preset entries must be absolute paths (or ~/…)'); + return mutate(_dedupe([...store.dirsFor(cwd), ...dirs])); + + case 'remove': + if (rawDirs.isEmpty) return _err(req.id, 'remove requires at least one '); + final (dirs, bad) = _expandAll(rawDirs, home()); + if (bad != null) return _err(req.id, bad, hint: 'pass the entry exactly as `env path list` shows it'); + final current = store.dirsFor(cwd); + final drop = dirs.toSet(); + final kept = current.where((d) => !drop.contains(d)).toList(); + if (kept.length == current.length) { + return _err(req.id, 'no preset entry matches ${dirs.join(", ")}', hint: 'clide env path list'); + } + return mutate(kept); + + case 'clear': + return mutate(const []); + + case 'capture': + final dirs = store.dirsFor(cwd); + final login = loginPath(); + final proc = processPath(); + final suggested = missingLoginShellDirs(loginPath: login, processPath: proc).where((d) => !dirs.contains(d)).toList(); + return _ok(req.id, { + 'root': root, + 'suggested': suggested, + 'loginShellPath': login, + 'processPath': proc, + if (login == null) 'note': 'login-shell PATH probe unavailable; nothing to diff', + }); + + default: + return _err( + req.id, + 'unknown env path action: ${action == null || action.isEmpty ? '(none)' : action}', + hint: 'use: list | set | add | remove | clear | capture', + ); + } +} + +/// Expand a leading `~/` against [home] and require absolute results. Returns +/// the expanded list, or an error message on the first bad entry. +(List, String?) _expandAll(List dirs, String? home) { + final out = []; + for (final raw in dirs) { + var d = raw.trim(); + if (d.isEmpty) return (out, 'empty preset entry'); + if (d == '~' || d.startsWith('~/')) { + if (home == null || home.isEmpty) return (out, 'cannot expand "~" (no HOME)'); + d = d == '~' ? home : '$home${d.substring(1)}'; + } + final absolute = d.startsWith('/') || RegExp(r'^[A-Za-z]:[/\\]').hasMatch(d); + if (!absolute) return (out, 'not an absolute path: $raw'); + while (d.length > 1 && d.endsWith('/')) { + d = d.substring(0, d.length - 1); + } + out.add(d); + } + return (out, null); +} + +List _dedupe(List dirs) { + final out = []; + for (final d in dirs) { + if (!out.contains(d)) out.add(d); + } + return out; +} + +List _missing(List dirs, bool Function(String) dirExists) => [ + for (final d in dirs) + if (!dirExists(d)) d, +]; + +IpcResponse _ok(String id, Map data) => IpcResponse.ok(id: id, data: data); + +IpcResponse _err(String id, String message, {String? hint}) => IpcResponse.err( + id: id, + error: IpcError(code: IpcExitCode.userError, kind: IpcErrorKind.userError, message: message, hint: hint), +); diff --git a/lib/src/env/path_preset.dart b/lib/src/env/path_preset.dart new file mode 100644 index 00000000..c0c33499 --- /dev/null +++ b/lib/src/env/path_preset.dart @@ -0,0 +1,144 @@ +/// Per-workspace PATH preset (D-106, T-511) — a machine-local, user-scope list +/// of directories prepended to the PATH clide hands to every shell it spawns: +/// the hosted Claude session (so the agent's Bash tool sees it) and terminal +/// PTY panes. The T-439 login-shell probe is a global heuristic with a known +/// hole (interactive-only profile additions like brew shellenv in `~/.bashrc`); +/// the preset is the explicit per-repo layer on top of it. +/// +/// Storage is the app (user) settings layer keyed by workspace hash — the +/// account-binding pattern (T-483) — so absolute machine paths never land in a +/// committed file. The hash keys off the REPO identity, not the literal +/// directory: a linked git worktree (its `.git` is a `gitdir:` pointer file, +/// e.g. under an in-repo `.worktrees/` dir) resolves to the main repo root, so +/// every worktree shares its repo's preset. +/// +/// Security fence: the preset feeds spawned-shell environments only. Clide's +/// own binary resolution (D-104 pins, bundled pql/git, the toolchain probe) +/// never consults it — see D-106. +/// +/// Flutter-free (consumed by the `env path` daemon verbs under `dart test`). +library; + +import 'dart:io'; + +import '../ipc/paths.dart' show canonicalWorkspaceKey, fnv1a64Hex; + +/// Prefix of the user-scope settings key; the suffix is the preset-root hash. +const pathPresetKeyPrefix = 'app.env.pathPrepend.'; + +/// The settings key holding [workspaceRoot]'s preset — app layer, suffixed +/// with the FNV-1a hash (the same one D-70 derives for the socket path) of the +/// [presetRootFor]-resolved repo root, so all worktrees of a repo share one key. +String pathPresetKey(String workspaceRoot, {bool Function(String path)? isFile, String? Function(String path)? readFile}) => + '$pathPresetKeyPrefix${fnv1a64Hex(canonicalWorkspaceKey(presetRootFor(workspaceRoot, isFile: isFile, readFile: readFile)))}'; + +/// The directory whose identity keys the preset: the MAIN repo root when +/// [workspaceRoot] is a linked git worktree, else [workspaceRoot] itself +/// (trailing separators stripped, so `/repo` and `/repo/` map alike). +/// +/// A linked worktree's `.git` is a pointer FILE — `gitdir:
/.git/worktrees/` +/// (git writes forward slashes on every platform; a relative +/// gitdir is resolved against the worktree root). Anything that doesn't match +/// that shape — a normal repo (`.git` directory), no `.git` at all, a +/// submodule pointer — keys off [workspaceRoot] unchanged. +/// +/// [isFile]/[readFile] are injectable for tests; defaults touch the real fs. +String presetRootFor(String workspaceRoot, {bool Function(String path)? isFile, String? Function(String path)? readFile}) { + final root = _stripTrailingSep(workspaceRoot); + final probe = isFile ?? _isFile; + final read = readFile ?? _readFile; + final gitPointer = '$root/.git'; + if (!probe(gitPointer)) return root; + final content = read(gitPointer); + if (content == null) return root; + final match = RegExp(r'^gitdir:\s*(.+?)\s*$', multiLine: true).firstMatch(content); + if (match == null) return root; + final gitdir = match.group(1)!.replaceAll(r'\', '/'); + final resolved = _normalize(_isAbsolute(gitdir) ? gitdir : '${root.replaceAll(r'\', '/')}/$gitdir'); + const marker = '/.git/worktrees/'; + final idx = resolved.indexOf(marker); + if (idx <= 0) return root; + return resolved.substring(0, idx); +} + +/// Read [workspaceRoot]'s preset through an injected settings [read] (key → +/// stored value). Tolerant of a malformed value: anything that isn't a list of +/// non-empty strings is skipped, mirroring `AccountRegistry.accounts`. +List presetDirsFrom( + Object? Function(String key) read, + String workspaceRoot, { + bool Function(String path)? isFile, + String? Function(String path)? readFile, +}) { + final raw = read(pathPresetKey(workspaceRoot, isFile: isFile, readFile: readFile)); + if (raw is! List) return const []; + return [ + for (final e in raw) + if (e is String && e.trim().isNotEmpty) e, + ]; +} + +/// Pure prepend: [preset] dirs (de-duplicated, order kept) ahead of [base], +/// with base entries that repeat a preset dir dropped so the preset always +/// wins. An empty preset returns [base] unchanged. +String applyPathPreset(String base, List preset, {String sep = ':'}) { + final dirs = []; + for (final d in preset) { + final t = d.trim(); + if (t.isNotEmpty && !dirs.contains(t)) dirs.add(t); + } + if (dirs.isEmpty) return base; + final baseParts = base.isEmpty ? const [] : base.split(sep); + return [...dirs, ...baseParts.where((p) => !dirs.contains(p))].join(sep); +} + +/// Capture-from-login-shell diff (D-106's `env path capture`): the entries the +/// login-shell PATH has that the process PATH lacks — the dirs a desktop +/// launch dropped, i.e. the preset candidates. Empty when the probe failed +/// ([loginPath] null/empty). Order-preserved, de-duplicated. +List missingLoginShellDirs({required String? loginPath, required String processPath, String sep = ':'}) { + if (loginPath == null || loginPath.isEmpty) return const []; + final have = processPath.split(sep).toSet(); + final out = []; + for (final d in loginPath.split(sep)) { + if (d.isNotEmpty && !have.contains(d) && !out.contains(d)) out.add(d); + } + return out; +} + +bool _isFile(String path) => FileSystemEntity.typeSync(path) == FileSystemEntityType.file; + +String? _readFile(String path) { + try { + return File(path).readAsStringSync(); + } catch (_) { + return null; + } +} + +bool _isAbsolute(String p) => p.startsWith('/') || RegExp(r'^[A-Za-z]:/').hasMatch(p); + +/// Segment-wise `.`/`..` normalization over forward-slash paths (a gitdir +/// pointer is often relative, e.g. `../../.git/worktrees/x`). No fs access. +String _normalize(String p) { + final drive = RegExp(r'^[A-Za-z]:').firstMatch(p)?.group(0) ?? ''; + final rest = p.substring(drive.length); + final out = []; + for (final seg in rest.split('/')) { + if (seg.isEmpty || seg == '.') continue; + if (seg == '..') { + if (out.isNotEmpty) out.removeLast(); + continue; + } + out.add(seg); + } + return '$drive/${out.join('/')}'; +} + +String _stripTrailingSep(String p) { + var s = p; + while (s.length > 1 && (s.endsWith('/') || s.endsWith(r'\'))) { + s = s.substring(0, s.length - 1); + } + return s; +} diff --git a/lib/src/env/shell_env.dart b/lib/src/env/shell_env.dart index 9d47fd82..9a7d1c51 100644 --- a/lib/src/env/shell_env.dart +++ b/lib/src/env/shell_env.dart @@ -80,6 +80,11 @@ String expandToolPath(String base, {required bool isMac, required bool isLinux, return [...missing, ...existing].join(':'); } +/// The raw login-shell PATH the probe captured, or null when it is +/// unavailable (probe failed / not yet primed / Windows). `env path capture` +/// (D-106) diffs this against the process PATH to suggest preset entries. +String? loginShellPathOrNull() => _loginShellPath; + /// Test seam: force the cached login-shell PATH (and mark primed). void debugSetLoginShellPath(String? value) { _loginShellPath = value; diff --git a/lib/src/panes/registry.dart b/lib/src/panes/registry.dart index f4b015c6..e6be9a4c 100644 --- a/lib/src/panes/registry.dart +++ b/lib/src/panes/registry.dart @@ -19,10 +19,18 @@ import 'event_sink.dart'; import 'pane.dart'; class PaneRegistry { - PaneRegistry({required this.events, this.ptyLog = PtyLog.none}); + PaneRegistry({required this.events, this.ptyLog = PtyLog.none, String Function(String? cwd)? pathForSpawn}) + : _pathForSpawn = pathForSpawn ?? _defaultPathForSpawn; final DaemonEventSink events; + /// PATH for a child spawned in `cwd` — the seam the per-workspace PATH + /// preset (D-106) is applied through; the default is the plain resolved + /// login-shell PATH (T-439). main.dart wires the preset-aware closure. + final String Function(String? cwd) _pathForSpawn; + + static String _defaultPathForSpawn(String? _) => resolvedToolPath(); + /// Breadcrumb hook handed to every PTY this registry spawns (T-434). Default /// no-op; production wires it to the kernel Logger + a crumb file. final PtyLog ptyLog; @@ -55,8 +63,9 @@ class PaneRegistry { 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(), + // even on a desktop launch (T-439), plus the workspace's PATH preset + // (D-106) via [_pathForSpawn]; an explicit caller PATH still wins. + 'PATH': _pathForSpawn(cwd), 'TERM': 'xterm-256color', 'COLORTERM': 'truecolor', 'LANG': 'en_US.UTF-8', diff --git a/test/builtin/claude/agent_bootstrap_test.dart b/test/builtin/claude/agent_bootstrap_test.dart index a9690fdb..bc960bb5 100644 --- a/test/builtin/claude/agent_bootstrap_test.dart +++ b/test/builtin/claude/agent_bootstrap_test.dart @@ -67,6 +67,33 @@ void main() { final d = agentEnvDelta(workspaceRoot: '/repo', socketPath: '/s.sock', currentPath: null, clideCliDir: '/opt/clide/bin'); expect(d['PATH'], '/opt/clide/bin'); }); + + test('exports PATH for a preset even when clide is already resolvable (D-106)', () { + final d = agentEnvDelta(workspaceRoot: '/repo', socketPath: '/s.sock', currentPath: '/usr/bin:/bin', clideCliDir: null, prependDirs: ['/opt/go/bin']); + expect(d['PATH'], '/opt/go/bin:/usr/bin:/bin'); + }); + + test('preset dirs come first, then the cli dir, then the current PATH (D-106)', () { + final d = agentEnvDelta( + workspaceRoot: '/repo', + socketPath: '/s.sock', + currentPath: '/usr/bin', + clideCliDir: '/home/dev/.local/bin', + prependDirs: ['/opt/go/bin', '/brew/bin'], + ); + expect(d['PATH'], '/opt/go/bin:/brew/bin:/home/dev/.local/bin:/usr/bin'); + }); + + test('a preset dir already on the current PATH is not duplicated (D-106)', () { + final d = agentEnvDelta( + workspaceRoot: '/repo', + socketPath: '/s.sock', + currentPath: '/opt/go/bin:/usr/bin', + clideCliDir: null, + prependDirs: ['/opt/go/bin'], + ); + expect(d['PATH'], '/opt/go/bin:/usr/bin'); + }); }); group('resolveClideCliDir (T-215)', () { @@ -143,5 +170,16 @@ void main() { final b = agentBootstrap('/ws', base: {'CLAUDE_CONFIG_DIR': '/override'}, boundConfigDir: (_) => '/bound'); expect(b.envDelta['CLAUDE_CONFIG_DIR'], '/override'); }); + + test('the workspace PATH preset lands at the head of the delta PATH (D-106)', () { + final b = agentBootstrap('/ws', pathPreset: (cwd) => cwd == '/ws' ? ['/opt/go/bin'] : const []); + expect(b.envDelta['PATH'], startsWith('/opt/go/bin:')); + }); + + test('no preset wired → bootstrap behaves as before (no gratuitous PATH export)', () { + final emptyPreset = agentBootstrap('/ws', pathPreset: (_) => const []); + final unwired = agentBootstrap('/ws'); + expect(emptyPreset.envDelta.containsKey('PATH'), unwired.envDelta.containsKey('PATH')); + }); }); } diff --git a/test/daemon/env_path_commands_test.dart b/test/daemon/env_path_commands_test.dart new file mode 100644 index 00000000..073baee9 --- /dev/null +++ b/test/daemon/env_path_commands_test.dart @@ -0,0 +1,182 @@ +/// Tests for `env path …` (D-106, T-511; D-6 CLI parity). Verifies each verb's +/// store effect, the published mutation payloads, the capture diff, and honest +/// userErrors — against a fake PathPresetStore so the handler stays +/// Flutter-free (runs under `dart test`). +library; + +import 'package:clide/clide.dart'; +import 'package:clide/src/daemon/env_path_commands.dart'; +import 'package:test/test.dart'; + +class _FakeStore implements PathPresetStore { + final Map> byCwd = {}; + + @override + List dirsFor(String cwd) => List.of(byCwd[cwd] ?? const []); + + @override + Future setFor(String cwd, List dirs) async { + if (dirs.isEmpty) { + byCwd.remove(cwd); + } else { + byCwd[cwd] = List.of(dirs); + } + } +} + +void main() { + late _FakeStore store; + late List<({String publisher, String channel, Map data})> published; + late DaemonDispatcher d; + late Set existingDirs; + String? loginPath; + var processPath = '/usr/bin:/bin'; + + void wire({String? cwd = '/repo', bool withStore = true}) { + store = _FakeStore(); + published = []; + existingDirs = {}; + loginPath = null; + processPath = '/usr/bin:/bin'; + d = DaemonDispatcher(); + registerEnvPathCommands( + d, + () => withStore ? store : null, + publisher: () => + (p, c, data) => published.add((publisher: p, channel: c, data: data)), + workspaceCwd: () => cwd, + home: () => '/home/u', + dirExists: (dir) => existingDirs.contains(dir), + loginPath: () => loginPath, + processPath: () => processPath, + ); + } + + Future run(List positional) => d.dispatch(IpcRequest(id: '1', cmd: 'env.path', args: {'positional': positional})); + + test('registered on the dispatcher → shows in capabilities', () async { + wire(); + final caps = await d.dispatch(IpcRequest(id: 'c', cmd: 'capabilities', args: const {})); + expect((caps.data['commands'] as Map).containsKey('env.path'), isTrue); + }); + + group('set', () { + test('replaces the preset, de-duplicated, and publishes', () async { + wire(); + existingDirs.add('/opt/go/bin'); + final r = await run(['set', '/opt/go/bin', '/x', '/opt/go/bin']); + expect(r.ok, isTrue, reason: r.error?.message); + expect(r.data['dirs'], ['/opt/go/bin', '/x']); + expect(r.data['missing'], ['/x'], reason: 'nonexistent dirs warn, never error'); + expect(store.byCwd['/repo'], ['/opt/go/bin', '/x']); + expect(published.single.channel, envPathChannel); + expect(published.single.data['action'], 'set'); + expect(published.single.data['dirs'], ['/opt/go/bin', '/x']); + }); + + test('expands ~/ against HOME and strips trailing slashes', () async { + wire(); + final r = await run(['set', '~/go/bin/', '~']); + expect(r.ok, isTrue, reason: r.error?.message); + expect(r.data['dirs'], ['/home/u/go/bin', '/home/u']); + }); + + test('relative paths error; empty set points at clear', () async { + wire(); + final rel = await run(['set', 'go/bin']); + expect(rel.ok, isFalse); + expect(rel.error?.message, contains('not an absolute path')); + final empty = await run(['set']); + expect(empty.ok, isFalse); + expect(empty.error?.hint, contains('clear')); + }); + + test('a leading-dash entry is rejected by the schema (T-104 guard)', () async { + wire(); + final r = await d.dispatch( + IpcRequest( + id: '1', + cmd: 'env.path', + args: { + 'positional': ['set'], + 'flags': {'dirs': '--evil'}, + }, + ), + ); + expect(r.ok, isFalse); + }); + }); + + group('add / remove / clear', () { + test('add appends without duplicating; remove drops; clear empties the key', () async { + wire(); + await run(['set', '/a']); + published.clear(); + + var r = await run(['add', '/b', '/a']); + expect(r.data['dirs'], ['/a', '/b']); + + r = await run(['remove', '/a']); + expect(r.data['dirs'], ['/b']); + + r = await run(['clear']); + expect(r.ok, isTrue); + expect(r.data['dirs'], isEmpty); + expect(store.byCwd.containsKey('/repo'), isFalse, reason: 'empty preset removes the key'); + expect(published.map((p) => p.data['action']), ['add', 'remove', 'clear']); + }); + + test('add and remove require a ; removing an absent entry errors', () async { + wire(); + expect((await run(['add'])).ok, isFalse); + expect((await run(['remove'])).ok, isFalse); + final r = await run(['remove', '/ghost']); + expect(r.ok, isFalse); + expect(r.error?.message, contains('no preset entry matches')); + }); + }); + + group('list', () { + test('returns the preset, the missing subset, and the effective PATH preview', () async { + wire(); + existingDirs.add('/a'); + await run(['set', '/a', '/gone']); + final r = await run(['list']); + expect(r.data['root'], '/repo'); + expect(r.data['dirs'], ['/a', '/gone']); + expect(r.data['missing'], ['/gone']); + expect(r.data['effectivePath'], startsWith('/a:/gone:'), reason: 'preset prepends the resolved PATH'); + }); + }); + + group('capture', () { + test('suggests login-shell dirs the process PATH lacks, minus the preset', () async { + wire(); + loginPath = '/brew/bin:/usr/bin:/opt/go/bin:/bin'; + await run(['set', '/brew/bin']); + final r = await run(['capture']); + expect(r.ok, isTrue, reason: r.error?.message); + expect(r.data['suggested'], ['/opt/go/bin']); + expect(r.data['loginShellPath'], loginPath); + expect(r.data['processPath'], '/usr/bin:/bin'); + }); + + test('no login-shell probe → empty suggestions with an honest note', () async { + wire(); + final r = await run(['capture']); + expect(r.data['suggested'], isEmpty); + expect(r.data['note'], contains('unavailable')); + }); + }); + + test('no workspace / no store / unknown action error clearly', () async { + wire(cwd: null); + expect((await run(['list'])).error?.message, contains('no workspace')); + wire(withStore: false); + expect((await run(['list'])).error?.message, contains('unavailable')); + wire(); + final r = await run(['frobnicate']); + expect(r.ok, isFalse); + expect(r.error?.hint, contains('list | set | add | remove | clear | capture')); + }); +} diff --git a/test/panes/registry_test.dart b/test/panes/registry_test.dart index 73d076a3..ae634f12 100644 --- a/test/panes/registry_test.dart +++ b/test/panes/registry_test.dart @@ -99,6 +99,32 @@ void main() { expect(pane.kind, PaneKind.claude); expect(pane.toJson()['kind'], 'claude'); }); + + test('pathForSpawn hook sets the child PATH per cwd (D-106)', tags: ['pty'], () async { + String? seenCwd; + final preset = PaneRegistry( + events: sink, + pathForSpawn: (cwd) { + seenCwd = cwd; + return '/preset-marker:/usr/bin:/bin'; + }, + ); + addTearDown(preset.shutdown); + + final buf = StringBuffer(); + final got = Completer(); + final sub = sink.stream.listen((e) { + if (e.kind != 'pane.output') return; + buf.write(utf8.decode(base64Decode(e.data['bytes_b64']! as String))); + if (buf.toString().contains('/preset-marker') && !got.isCompleted) got.complete(); + }); + addTearDown(sub.cancel); + + await preset.spawn(kind: PaneKind.terminal, argv: const ['/bin/sh', '-c', r'printf %s "$PATH"; sleep 0.25'], cwd: '/tmp'); + + await got.future.timeout(ioTimeout, onTimeout: () => fail('child PATH never carried the preset marker within ${ioTimeout.inSeconds}s')); + expect(seenCwd, '/tmp'); + }); }); group('RecordingEventSink filters', () { diff --git a/test/src/env/path_preset_test.dart b/test/src/env/path_preset_test.dart new file mode 100644 index 00000000..1e3fc668 --- /dev/null +++ b/test/src/env/path_preset_test.dart @@ -0,0 +1,139 @@ +/// Tests for the per-workspace PATH preset (D-106, T-511): the pure prepend / +/// capture-diff helpers, the worktree-aware preset root, and the settings-key +/// derivation. Runs under plain `dart test` (Flutter-free). +library; + +import 'dart:io'; + +import 'package:clide/src/env/path_preset.dart'; +import 'package:test/test.dart'; + +void main() { + group('applyPathPreset', () { + test('prepends preset dirs ahead of the base', () { + expect(applyPathPreset('/usr/bin:/bin', ['/opt/go/bin', '/x']), '/opt/go/bin:/x:/usr/bin:/bin'); + }); + + test('empty preset returns the base unchanged', () { + expect(applyPathPreset('/usr/bin', const []), '/usr/bin'); + expect(applyPathPreset('/usr/bin', ['', ' ']), '/usr/bin'); + }); + + test('empty base returns just the preset', () { + expect(applyPathPreset('', ['/a', '/b']), '/a:/b'); + }); + + test('de-duplicates within the preset, order kept', () { + expect(applyPathPreset('/bin', ['/a', '/b', '/a']), '/a:/b:/bin'); + }); + + test('drops base entries the preset already names (preset wins)', () { + expect(applyPathPreset('/usr/bin:/a:/bin', ['/a']), '/a:/usr/bin:/bin'); + }); + + test('honours a custom separator', () { + expect(applyPathPreset(r'C:\bin', [r'C:\go'], sep: ';'), r'C:\go;C:\bin'); + }); + }); + + group('missingLoginShellDirs', () { + test('null or empty login PATH → nothing to suggest', () { + expect(missingLoginShellDirs(loginPath: null, processPath: '/bin'), isEmpty); + expect(missingLoginShellDirs(loginPath: '', processPath: '/bin'), isEmpty); + }); + + test('suggests login entries the process PATH lacks, order kept', () { + expect(missingLoginShellDirs(loginPath: '/home/linuxbrew/.linuxbrew/bin:/usr/bin:/opt/go/bin:/bin', processPath: '/usr/bin:/bin'), [ + '/home/linuxbrew/.linuxbrew/bin', + '/opt/go/bin', + ]); + }); + + test('de-duplicates and skips empty segments', () { + expect(missingLoginShellDirs(loginPath: '/a::/a:/b', processPath: '/bin'), ['/a', '/b']); + }); + }); + + group('presetRootFor (worktree resolution)', () { + test('a normal repo (.git directory) keys off itself', () { + expect(presetRootFor('/repo', isFile: (_) => false, readFile: (_) => fail('not read')), '/repo'); + }); + + test('trailing separators are stripped', () { + expect(presetRootFor('/repo//', isFile: (_) => false, readFile: (_) => null), '/repo'); + }); + + test('an in-repo .worktrees worktree resolves to the main repo root (absolute gitdir)', () { + expect( + presetRootFor('/repo/.worktrees/fix', isFile: (p) => p == '/repo/.worktrees/fix/.git', readFile: (p) => 'gitdir: /repo/.git/worktrees/fix\n'), + '/repo', + ); + }); + + test('a relative gitdir pointer resolves against the worktree root', () { + expect( + presetRootFor('/repo/.worktrees/fix', isFile: (p) => p == '/repo/.worktrees/fix/.git', readFile: (p) => 'gitdir: ../../.git/worktrees/fix'), + '/repo', + ); + }); + + test('a worktree outside the repo still resolves to the main root', () { + expect(presetRootFor('/tmp/wt', isFile: (p) => p == '/tmp/wt/.git', readFile: (_) => 'gitdir: /srv/repos/main/.git/worktrees/wt'), '/srv/repos/main'); + }); + + test('a gitdir pointer without the worktrees marker (submodule-style) keys off itself', () { + expect(presetRootFor('/repo/sub', isFile: (p) => p == '/repo/sub/.git', readFile: (_) => 'gitdir: /repo/.git/modules/sub'), '/repo/sub'); + }); + + test('malformed or unreadable pointer files key off the workspace itself', () { + expect(presetRootFor('/w', isFile: (_) => true, readFile: (_) => 'not a pointer'), '/w'); + expect(presetRootFor('/w', isFile: (_) => true, readFile: (_) => null), '/w'); + }); + + test('backslashed gitdir (Windows-written pointer) still matches', () { + expect(presetRootFor('/repo/.worktrees/x', isFile: (p) => p == '/repo/.worktrees/x/.git', readFile: (_) => r'gitdir: /repo/.git\worktrees\x'), '/repo'); + }); + + test('resolves a REAL worktree layout on disk (no injected probes)', () { + final tmp = Directory.systemTemp.createTempSync('preset-root'); + addTearDown(() => tmp.deleteSync(recursive: true)); + final repo = Directory('${tmp.path}/repo')..createSync(); + Directory('${repo.path}/.git/worktrees/fix').createSync(recursive: true); + final wt = Directory('${repo.path}/.worktrees/fix')..createSync(recursive: true); + File('${wt.path}/.git').writeAsStringSync('gitdir: ${repo.path}/.git/worktrees/fix\n'); + expect(presetRootFor(wt.path), repo.path); + expect(presetRootFor(repo.path), repo.path); + }); + }); + + group('pathPresetKey', () { + test('is an app-layer key under the preset prefix', () { + final key = pathPresetKey('/repo', isFile: (_) => false, readFile: (_) => null); + expect(key, startsWith(pathPresetKeyPrefix)); + expect(key.length, pathPresetKeyPrefix.length + 16, reason: '16-hex FNV-1a suffix'); + }); + + test('a worktree and its main repo share one key; trailing slash is unified', () { + final main = pathPresetKey('/repo', isFile: (_) => false, readFile: (_) => null); + final slash = pathPresetKey('/repo/', isFile: (_) => false, readFile: (_) => null); + final wt = pathPresetKey('/repo/.worktrees/fix', isFile: (p) => p == '/repo/.worktrees/fix/.git', readFile: (_) => 'gitdir: /repo/.git/worktrees/fix'); + expect(slash, main); + expect(wt, main); + expect(pathPresetKey('/other', isFile: (_) => false, readFile: (_) => null), isNot(main)); + }); + }); + + group('presetDirsFrom', () { + List read(Object? stored) => presetDirsFrom((_) => stored, '/repo', isFile: (_) => false, readFile: (_) => null); + + test('reads a stored list of dirs', () { + expect(read(['/a', '/b']), ['/a', '/b']); + }); + + test('tolerates malformed values', () { + expect(read(null), isEmpty); + expect(read('nonsense'), isEmpty); + expect(read([1, '', ' ', '/ok', true]), ['/ok']); + }); + }); +}