Start a session in the user's home, from one place - #116
Merged
Merged
Conversation
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) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_01KdG8bDrddXkEkzgPq23D6q
Codecov Report❌ Patch coverage is
📢 Thoughts on this report? Let us know! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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— whichis nobody's idea of a starting point.
The behaviour existed, in the wrong place
omnyshell node startalready passedworkingDirectory: home(
bin/omnyshell.dart), but it did so in the CLI rather than in the backend.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.
The fix
resolveStartDirectorynow holds the decision, in the order of who is entitledto make it:
~expanded; a directory that does not existstill fails loudly, which is the right answer to an explicit request that
cannot be honoured.
existingUserHome, so it works on aservice-run node that was handed no
HOME(1.57.1).null— inherit the node's own directory, as before.execfollows the same rule as an interactive shell, so the two agree;sshset that expectation long ago.
A home that does not exist is skipped rather than used: handing a missing
path to
Process.startfails the session outright, which is worse than openingsomewhere unremarkable. A blank string falls through for the same reason.
Drift this turned up
Consolidating the three backends onto one function exposed that
ScriptPtyShellBackend— the default on Linux and macOS — never expanded aleading
~. So--cwd ~/projectworked on the pipe and winpty backends andsilently did not on the one nearly everyone uses. They share the resolution now
and cannot diverge again.
The Windows MSYS path translation was duplicated in two backends; it is applied
once, where the decision is made.
_resolveNodeHomekeeps only what is particular to it — the node profile'sHOMEoverriding the process environment — and defers the rest toexistingUserHome, so the password-database lookup a service-run node needs isnot reimplemented beside it.
Verified
The new backend test builds a bare
ProcessShellBackend()from a process whosecwd is the repo, and both an
execand an interactive session report the homedirectory — the embedder shape that had nothing before. Unit tests cover the
precedence chain, the blank-string fall-through, the
~expansion and therefusal to use a home that is not on disk.
696 unit and 136 integration tests pass;
dart analyzeclean.v1.57.1is tagged, so this carries the bump to 1.57.2 —pubspec.yamlandlib/src/version.dart, pinned together byversion_test.dart— plus itschangelog section.
🤖 Generated with Claude Code
https://claude.ai/code/session_01KdG8bDrddXkEkzgPq23D6q