From fd1b2a410cc542475c8ac0d6c5fcdc7adcf19521 Mon Sep 17 00:00:00 2001 From: "Graciliano M. P." Date: Sun, 13 Sep 2026 04:36:10 -0300 Subject: [PATCH] Start a session in the user's home, from one place MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A shell opened wherever the node process happened to be standing. For an agent installed as a service that is where its binary lives — `/usr/local/bin` — which is nobody's idea of a starting point. The behaviour existed: `omnyshell node start` passed `workingDirectory: home`. It did so in the CLI rather than in the backend, so an embedder building its own ProcessShellBackend — as OmnyServer does — got neither that nor anything else. A default that only one caller applies is not a default. resolveStartDirectory now holds the decision, in the order of who is entitled to make it: the client's request, then the node's configuration, then the user's home, then the node's own directory. exec follows the same rule as an interactive shell, so the two agree; ssh set that expectation long ago. A home that does not exist is skipped rather than used — handing a missing path to Process.start fails the session outright, which is worse than opening somewhere unremarkable, and a blank string falls through for the same reason. Moving it also closed the drift between the three backends. The script PTY backend — the default on Linux and macOS — never expanded a leading `~`, so `--cwd ~/project` worked on the pipe and winpty backends and not on it. The Windows MSYS translation was duplicated in two of them and is now applied once, where the decision is made. `_resolveNodeHome` keeps only what is particular to it (the node profile's HOME overriding the environment) and defers the rest to existingUserHome, so the password-database lookup a service-run node needs is not reimplemented beside it. Verified where it counts: the new backend test starts a bare ProcessShellBackend from a process whose cwd is the repo, and both an exec and an interactive session report the home directory — the embedder path that had nothing before. v1.57.1 is tagged, so this carries the bump to 1.57.2 and its changelog. Co-Authored-By: Claude Opus 5 (1M context) Claude-Session: https://claude.ai/code/session_01KdG8bDrddXkEkzgPq23D6q --- CHANGELOG.md | 37 +++++++++++ bin/omnyshell.dart | 22 +++---- .../backend/process_shell_backend.dart | 16 ++--- .../backend/pty/script_pty_shell_backend.dart | 5 +- .../backend/pty/winpty_shell_backend.dart | 12 ++-- .../backend/shell_invocation.dart | 33 ++++++++++ lib/src/shared/utils/omnyshell_home.dart | 19 ++++++ lib/src/version.dart | 2 +- pubspec.yaml | 2 +- .../backend/process_shell_backend_test.dart | 24 +++++++ test/unit/backend/shell_invocation_test.dart | 66 +++++++++++++++++++ 11 files changed, 209 insertions(+), 29 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 76c4471..d317270 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,3 +1,40 @@ +## 1.57.2 + +A shell now opens where you would expect it to: the user's home. + +### Fixed + +- **A session starts in the user's home, not wherever the node was launched.** + `omnyshell node start` already did this — but in the CLI rather than in the + backend, so any embedder building its own `ProcessShellBackend` (as + OmnyServer does) got neither. Sessions opened in the node process's own + working directory, which for an agent installed as a service is wherever its + binary lives: `/usr/local/bin`. + + The decision now lives in one place, `resolveStartDirectory`, in the order + that matches who is entitled to make it: what the client asked for, then what + the node was configured with, then the user's home, and only then the node's + own directory. `exec` follows the same rule as an interactive shell, so the + two agree — and `ssh` set that expectation long ago. + + A home that does not exist is skipped rather than used: handing a missing + path to `Process.start` fails the session outright, which is worse than + opening somewhere unremarkable. + +### Changed + +- All three backends (pipe, `script` PTY, winpty) resolve their working + directory through that one function. They had drifted: the `script` PTY + backend — the default on Linux and macOS — never expanded a leading `~`, so + `--cwd ~/project` worked on the other two and not on it. The Windows MSYS + path translation was duplicated across two of them, and is now applied once, + where the decision is made. + +- `_resolveNodeHome` in the CLI keeps only what is particular to it — letting + the node profile's `HOME` override the process environment — and defers the + rest to `existingUserHome`, so the password-database lookup a service-run + node needs is not reimplemented beside it. + ## 1.57.1 A shell on a node run as a service had no `$HOME`. diff --git a/bin/omnyshell.dart b/bin/omnyshell.dart index 4e1f99f..503415d 100644 --- a/bin/omnyshell.dart +++ b/bin/omnyshell.dart @@ -29,6 +29,7 @@ import 'package:omnyshell/src/domain/entities/platform_info_io.dart'; import 'package:omnyshell/src/infrastructure/auth/node_git_credentials.dart'; import 'package:omnyshell/src/infrastructure/identity/certificate_names.dart'; import 'package:omnyshell/src/infrastructure/tls/ca_pinning.dart'; +import 'package:omnyshell/src/shared/utils/omnyshell_home.dart'; Future main(List args) async { final runner = @@ -2088,18 +2089,15 @@ String _resolveNodeShell(String? override) { return (shell != null && shell.trim().isNotEmpty) ? shell : '/bin/sh'; } -/// Resolves the node user's home directory — preferring the profile's `HOME`, -/// then the node process environment — used as the default working directory of -/// new sessions. Returns `null` when it cannot be resolved or does not exist, so -/// sessions fall back to the node's own cwd. -String? _resolveNodeHome(Map env) { - final home = - env['HOME'] ?? - Platform.environment['HOME'] ?? - Platform.environment['USERPROFILE']; - if (home == null || home.trim().isEmpty) return null; - return Directory(home).existsSync() ? home : null; -} +/// Resolves the node user's home directory, letting the node profile's `HOME` +/// override the process environment. +/// +/// The backends fall back to the user's home by themselves now, so this exists +/// only for that override. `existingUserHome` does the resolving — including +/// the password-database lookup a service-run node needs, and the check that +/// the directory is actually there. +String? _resolveNodeHome(Map env) => + existingUserHome(environment: {...Platform.environment, ...env}); /// Resolves the optional working-directory positional of `omnyshell local` and /// `omnyshell ide` ([command] names the one being run, for the error messages). diff --git a/lib/src/infrastructure/backend/process_shell_backend.dart b/lib/src/infrastructure/backend/process_shell_backend.dart index 00fc58c..ce8356b 100644 --- a/lib/src/infrastructure/backend/process_shell_backend.dart +++ b/lib/src/infrastructure/backend/process_shell_backend.dart @@ -50,16 +50,12 @@ class ProcessShellBackend implements ShellBackend { } final (executable, args) = resolveShellInvocation(request, defaultShell); - // Resolve a leading `~` (the client may pass `~/...` as the working dir, e.g. - // an ephemeral `run`/drive mount path) against the node user's home. On - // Windows, also translate an MSYS cwd (`/c/...`, as Git Bash reports `$PWD` - // and as TAB-completion's one-shot exec reuses it) into a Windows path - // `Process.start` can actually `chdir` into. - final raw = request.cwd; - var cwd = raw != null ? expandUserHome(raw) : workingDirectory; - if (Platform.isWindows && cwd != null && cwd.startsWith('/')) { - cwd = windowsPathFromMsys(cwd); - } + // Client's choice, then the node's, then the user's home — and only then + // wherever this process happens to be standing. See `resolveStartDirectory`. + final cwd = resolveStartDirectory( + requested: request.cwd, + configured: workingDirectory, + ); final process = await Process.start( executable, args, diff --git a/lib/src/infrastructure/backend/pty/script_pty_shell_backend.dart b/lib/src/infrastructure/backend/pty/script_pty_shell_backend.dart index e5555ee..04d6bef 100644 --- a/lib/src/infrastructure/backend/pty/script_pty_shell_backend.dart +++ b/lib/src/infrastructure/backend/pty/script_pty_shell_backend.dart @@ -93,7 +93,10 @@ class ScriptPtyShellBackend implements ShellBackend { final process = await Process.start( script, args, - workingDirectory: request.cwd ?? workingDirectory, + workingDirectory: resolveStartDirectory( + requested: request.cwd, + configured: workingDirectory, + ), // See `ProcessShellBackend`: a node run as a service has no `HOME`, and // a PTY session inherits that unless something fills it in. environment: withUserHome({ diff --git a/lib/src/infrastructure/backend/pty/winpty_shell_backend.dart b/lib/src/infrastructure/backend/pty/winpty_shell_backend.dart index 1cbb593..8ce8bf0 100644 --- a/lib/src/infrastructure/backend/pty/winpty_shell_backend.dart +++ b/lib/src/infrastructure/backend/pty/winpty_shell_backend.dart @@ -8,7 +8,6 @@ import '../../../domain/backend/pty_spec.dart'; import '../../../domain/backend/shell_backend.dart'; import '../../../domain/backend/shell_request.dart'; import '../../../domain/backend/shell_session.dart'; -import '../../../shared/utils/omnyshell_home.dart'; import '../shell_invocation.dart'; import 'winpty_ffi.dart'; import 'winpty_shell_session.dart'; @@ -105,9 +104,14 @@ class WinptyShellBackend implements ShellBackend { final inner = 'stty -echo 2>/dev/null; exec bash /dev/stdin'; final cmdline = '"$bash" -c "$inner"'; - // Resolve the working directory to a Windows path winpty can chdir into. - var cwd = expandUserHome(request.cwd ?? workingDirectory ?? ''); - if (cwd.startsWith('/')) cwd = windowsPathFromMsys(cwd); + // Client's choice, then the node's, then the user's home — already resolved + // to a path winpty can chdir into. See `resolveStartDirectory`. + final cwd = + resolveStartDirectory( + requested: request.cwd, + configured: workingDirectory, + ) ?? + ''; final env = { ...Platform.environment, diff --git a/lib/src/infrastructure/backend/shell_invocation.dart b/lib/src/infrastructure/backend/shell_invocation.dart index d26c5b3..5ee2bed 100644 --- a/lib/src/infrastructure/backend/shell_invocation.dart +++ b/lib/src/infrastructure/backend/shell_invocation.dart @@ -3,6 +3,7 @@ import 'dart:io'; import '../../domain/backend/shell_family.dart'; import '../../domain/backend/shell_request.dart'; import '../../domain/entities/session.dart'; +import '../../shared/utils/omnyshell_home.dart'; /// Resolves how a [ShellRequest] maps to an executable and arguments, shared by /// the pipe-based and PTY-based backends so both honour the same rules: @@ -145,6 +146,38 @@ String windowsPathFromMsys(String path) { return '$drive:\\${rest.replaceAll('/', r'\').replaceFirst(RegExp(r'^\\'), '')}'; } +/// The directory a session starts in, in order of who gets to decide. +/// +/// 1. [requested] — what the client asked for, which always wins. A leading `~` +/// is expanded; a directory that does not exist fails the spawn, loudly, +/// which is the right answer to an explicit request that cannot be honoured. +/// 2. [configured] — what the node was built with, for an embedder with an +/// opinion about where its sessions belong. +/// 3. The user's home. A shell that opens wherever the node process happens to +/// be standing — `/usr/local/bin`, for an agent installed as a service — is +/// nobody's idea of a starting point, and `ssh` set the expectation long +/// ago. Skipped when that directory does not exist: handing a missing path +/// to `Process.start` would fail the session outright. +/// 4. `null` — inherit the node's own working directory, as before. +/// +/// On Windows an MSYS path (`/c/Users/x`, the form Git Bash reports in `$PWD`) +/// is translated into one `chdir` accepts; see [windowsPathFromMsys]. +String? resolveStartDirectory({ + String? requested, + String? configured, + Map? environment, +}) { + final explicit = requested ?? configured; + final chosen = (explicit != null && explicit.trim().isNotEmpty) + ? expandUserHome(explicit) + : existingUserHome(environment: environment); + if (chosen == null) return null; + + return Platform.isWindows && chosen.startsWith('/') + ? windowsPathFromMsys(chosen) + : chosen; +} + /// The first *usable* Windows bash (Git Bash / WSL), or `null` when none works. /// Public shim over [_resolveWindowsBash] so the winpty backend can reuse the /// same validated bash the interactive session launches. diff --git a/lib/src/shared/utils/omnyshell_home.dart b/lib/src/shared/utils/omnyshell_home.dart index 74d74cb..396a593 100644 --- a/lib/src/shared/utils/omnyshell_home.dart +++ b/lib/src/shared/utils/omnyshell_home.dart @@ -78,6 +78,25 @@ int? _selfUid(String procStatusPath) { return null; } +/// The user's home directory, but only when it exists on disk. +/// +/// The distinction matters wherever the answer is about to be used as a +/// working directory: `Process.start` fails outright on one that is not there, +/// so a home that has been resolved but never created is worse than no answer. +String? existingUserHome({ + Map? environment, + String passwdPath = '/etc/passwd', + String procStatusPath = '/proc/self/status', +}) { + final home = resolveUserHome( + environment: environment, + passwdPath: passwdPath, + procStatusPath: procStatusPath, + ); + if (home == null || home.trim().isEmpty) return null; + return Directory(home).existsSync() ? home : null; +} + /// Returns [environment] with `HOME` filled in, when nothing else supplies one. /// /// A shell without `HOME` is subtly broken rather than obviously so: `cd ~` diff --git a/lib/src/version.dart b/lib/src/version.dart index 017c901..1a11ba5 100644 --- a/lib/src/version.dart +++ b/lib/src/version.dart @@ -3,4 +3,4 @@ /// This is the single source of truth for "what build is this": it is rendered /// in the CLI banner and is the default a node reports as its /// [NodeConfig.agentVersion] / [PlatformInfo.agentVersion]. -const String omnyShellVersion = '1.57.1'; +const String omnyShellVersion = '1.57.2'; diff --git a/pubspec.yaml b/pubspec.yaml index 6376e13..acaee30 100644 --- a/pubspec.yaml +++ b/pubspec.yaml @@ -4,7 +4,7 @@ description: >- connect to a Hub by node identity (not host:port); the Hub authenticates, authorizes and brokers encrypted sessions to Nodes over WebSocket-on-TLS. Ships Hub, Node, Client and CLI implementations behind first-class Dart APIs. -version: 1.57.1 +version: 1.57.2 repository: https://github.com/OmnyGrid/omnyshell environment: diff --git a/test/unit/backend/process_shell_backend_test.dart b/test/unit/backend/process_shell_backend_test.dart index f7147b9..44d9286 100644 --- a/test/unit/backend/process_shell_backend_test.dart +++ b/test/unit/backend/process_shell_backend_test.dart @@ -85,5 +85,29 @@ void main() { other.resolveSymbolicLinksSync(), ); }); + + // With nothing configured a session used to open wherever the node process + // happened to be standing — `/usr/local/bin` for an agent installed as a + // service, which is nobody's idea of a starting point. `ssh` set that + // expectation long ago, and `exec` follows the same rule so the two agree. + test('starts in the user home when nothing is configured', () async { + final home = Platform.environment['HOME']; + if (home == null || !Directory(home).existsSync()) { + markTestSkipped('no usable HOME in this environment'); + return; + } + + for (final mode in [SessionMode.exec, SessionMode.shell]) { + final session = await ProcessShellBackend().start( + ShellRequest(mode: mode, command: 'pwd'), + ); + final out = (await collect(session.stdout)).trim(); + expect( + Directory(out).resolveSymbolicLinksSync(), + Directory(home).resolveSymbolicLinksSync(), + reason: 'a $mode session should open at home', + ); + } + }); }, skip: Platform.isWindows ? 'POSIX shell semantics' : null); } diff --git a/test/unit/backend/shell_invocation_test.dart b/test/unit/backend/shell_invocation_test.dart index 67321f7..f2d4383 100644 --- a/test/unit/backend/shell_invocation_test.dart +++ b/test/unit/backend/shell_invocation_test.dart @@ -1,3 +1,5 @@ +import 'dart:io'; + import 'package:omnyshell/omnyshell.dart'; import 'package:omnyshell/src/infrastructure/backend/shell_invocation.dart'; import 'package:test/test.dart'; @@ -126,4 +128,68 @@ void main() { expect(args, const ['-v']); }); }); + + // Who decides where a session opens. Before this, an unconfigured backend + // left it to wherever the node process was standing — `/usr/local/bin` for an + // agent installed as a service. + group('resolveStartDirectory', () { + final home = Platform.environment['HOME']; + final hasHome = home != null && Directory(home).existsSync(); + + test('the client asked, so the client wins', () { + expect( + resolveStartDirectory(requested: '/srv/app', configured: '/opt/node'), + '/srv/app', + ); + }); + + test('then what the node was built with', () { + expect(resolveStartDirectory(configured: '/opt/node'), '/opt/node'); + }); + + test('a blank choice is no choice', () { + // An empty string handed to `Process.start` is not "the current + // directory", it is a failure — so it has to fall through like a null. + if (!hasHome) { + markTestSkipped('no usable HOME in this environment'); + return; + } + expect(resolveStartDirectory(configured: ' '), home); + expect(resolveStartDirectory(requested: ''), home); + }); + + test('otherwise the user home', () { + if (!hasHome) { + markTestSkipped('no usable HOME in this environment'); + return; + } + expect(resolveStartDirectory(), home); + }); + + test('but never a home that is not there', () { + // Guessing a missing directory into `Process.start` fails the session + // outright, which is worse than opening somewhere unremarkable. + final dir = Directory.systemTemp.createTempSync('omnyshell-nohome-'); + addTearDown(() => dir.deleteSync(recursive: true)); + + expect( + resolveStartDirectory( + environment: {'HOME': '${dir.path}/was-never-created'}, + ), + isNull, + reason: 'null means "inherit the node cwd", which at least exists', + ); + }); + + test('expands a leading ~ in an explicit choice', () { + if (!hasHome) { + markTestSkipped('no usable HOME in this environment'); + return; + } + expect( + resolveStartDirectory(requested: '~/projects'), + '$home${Platform.pathSeparator}projects', + ); + }); + }); }