iris/ai-app
1
0 Fork 0
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

Keep a typed path as typed, and expand ~ where it is used

Browse Source 
A llama session's tools all answered "failed to spawn process
[exit code: -1]": `llama-server` takes the working directory as an
`x-tool-cwd` header and `chdir`s to it with no shell in the way, so a
`~/…` cwd named a directory of that name. `files::resolve_blocking`
asks the machine that will serve the session what the path is, and the
driver does that once at launch.

The other half is the storing. `~` and `/home/someone` are a path and a
snapshot of where it pointed, and it is the snapshot that breaks when an
account is renamed -- so `machines::tidy` no longer expands one and
`set_cwd` no longer contracts one (`shorten_home` is gone with it). The
identity file is expanded at the point `ssh` is invoked instead.

Spawn now asks the same question of a typed working directory that
`set_cwd` already did: absolute or home-relative, and actually there on
the machine that will run it. It accepted anything, so a typo became a
session whose process could not start, reported later and pointing at
nothing.

Also: a provider written before `mcp_servers` existed adopts the
defaults a probe would give it now, so a machine discovered before
2026-09-19 stops silently having no web search.

Exercised end to end against a real llama session spawned with
`cwd: "~/repos/ai-app/server"`: `exec_shell_command` with `pwd` answered
`/home/bob/repos/ai-app/server`, exit 0.
This commit is contained in:
iris-ai committed 2026-09-20 17:06:13 -04:00
1 parent bd9596d782
commit 3b309766d7
8 files changed
+201 -101
Download Patch File Download Diff File Expand all files Collapse all files

No files matched your search

AGENTS.md
+16
View File
@@ -471,6 +471,22 @@ written, and the fold uses that same predicate to decide a reply is settled.
`generate` now fails the turn for both -- a reply that stops early is not a
reply, and the transcript keeps whatever arrived before it.
- **A path is stored as it was typed, and `~` is expanded where it is used.**
`~/repos/x` and `/home/someone/repos/x` are a path and a snapshot of where it
pointed, and the snapshot is what breaks when an account is renamed or the
value is read on another machine -- so nothing at the boundary rewrites one
in either direction (`machines::tidy` used to expand and `shorten_home` used
to contract; both are gone). Expansion belongs to the machine the path is on:
`ssh::quote_path` and `files::PATH_PRELUDE` for a remote one,
`ssh::expand_home` for one here. The exception that proves it is
**`llama-server`'s tools**, which take the working directory as an
`x-tool-cwd` header and `chdir` to it with no shell in the way: a `~` arrives
there as a directory of that name and *every* tool using one answers "failed
to spawn process\n[exit code: -1]", which on the phone looks like a session
whose tools are all broken. `files::resolve_blocking` is what the llama
driver resolves it with at launch, on the machine that will serve the
session.
- **A transcript outlives the enum.** Removing `Event::TaskNote` hours after
adding it made every transcript that had recorded one unreadable, so
`launch` failed for those sessions and `SessionManager::new` skipped them —
server/src/config.rs
+28 -1
View File
@@ -777,7 +777,11 @@ impl Config {
pub fn load(path: &Path) -> Result<Self> {
match std::fs::read_to_string(path) {
Ok(text) => format::parse(&text)
.with_context(|| format!("{} is not valid config RON", path.display())),
.with_context(|| format!("{} is not valid config RON", path.display()))
.map(|mut config| {
Self::adopt_mcp_defaults(&mut config);
config
}),
// A first run has no config -- the normal starting state; a token is
// generated and saved on that first start.
Err(err) if err.kind() == std::io::ErrorKind::NotFound => {
@@ -788,6 +792,29 @@ impl Config {
}
}
/// Gives a provider written before `mcp_servers` existed the defaults a
/// probe would give it now.
///
/// A migration, and a temporary one: the field arrived on 2026-09-19 and a
/// machine discovered before then has an empty list, which reads on screen
/// exactly like a machine somebody chose to give no MCP servers -- so a
/// llama session silently had no web search and nothing said why. Pressing
/// Rediscover fixes it, which is not something a person can be expected to
/// know. Delete this once every config here has been through it.
///
/// Empty rather than absent is the condition, because there is no way to
/// remove one from the phone: nothing can have chosen the empty list yet.
fn adopt_mcp_defaults(&mut self) {
for provider in self
.machines
.iter_mut()
.flat_map(|machine| machine.providers.iter_mut())
.filter(|provider| provider.mcp_servers.is_empty())
{
provider.mcp_servers = crate::machines::mcp_defaults(provider.kind);
}
}
/// Writes the config, owner-readable only.
///
/// The token hashes here are verifiers rather than secrets, but the file
server/src/files.rs
+52
View File
@@ -176,6 +176,34 @@ pub async fn list(transport: &Transport, path: &str) -> Result<Listing> {
})
}
/// The absolute path `path` names on that machine, with a leading `~` expanded
/// *there*.
///
/// What [`list`] answers as its `path`, asked on its own: a caller that has to
/// hand a directory to something with no shell in front of it needs the
/// resolved form and nothing else. `llama-server` is the one such caller --
/// its tools take a working directory as a request header and `chdir` to it
/// literally, so the `~` that every other path in this server carries through
/// to the far side's shell arrived there as a directory called `~`, and every
/// tool that uses one failed with "failed to spawn process".
///
/// Blocking because a driver's launch is, and this is a question for the
/// machine that will serve the session rather than for this one: a remote
/// `~` is the remote home, and expanding it here would name a directory on
/// the wrong machine -- which is also why the answer is not cached anywhere
/// but on the driver that asked.
pub fn resolve_blocking(transport: &Transport, path: &str) -> Result<String> {
let script = format!("{PATH_PRELUDE}cd -- \"$p\" && pwd -P");
let out = transport.capture_blocking(&launch(script, path, None))?;
// Only the newline `pwd` ends with: a directory name may legitimately end
// in a space, and trimming whitespace would rename it.
let resolved = out.trim_end_matches('\n');
if resolved.is_empty() {
anyhow::bail!("the machine did not say what {path} resolves to");
}
Ok(resolved.to_string())
}
/// The `find` output above, as rows. A record without all five fields is
/// dropped rather than guessed at: it can only come from a `find` that printed
/// something else, and half a row is worse than no row.
@@ -436,6 +464,30 @@ mod tests {
assert_eq!(names, ["binary.bin", "hello.txt", "it's a file", "sub"]);
}
/// The regression `resolve_blocking` exists for: a working directory typed
/// as `~/…` reaches `llama-server` as a header it `chdir`s to, so it has to
/// arrive absolute or every tool that uses one fails to spawn.
#[test]
fn a_tilde_resolves_to_that_machine_s_home() {
let Some(home) = std::env::home_dir() else {
return;
};
let resolved = resolve_blocking(&Transport::Here, "~").unwrap();
assert!(resolved.starts_with('/'), "{resolved}");
assert_eq!(
std::fs::canonicalize(&resolved).unwrap(),
std::fs::canonicalize(&home).unwrap(),
);
let dir = tree();
// An absolute path is answered as itself, resolved.
let full = dir.path().to_string_lossy().into_owned();
assert_eq!(
resolve_blocking(&Transport::Here, &full).unwrap(),
std::fs::canonicalize(&full).unwrap().to_string_lossy(),
);
assert!(resolve_blocking(&Transport::Here, &at(&dir, "nope")).is_err());
}
#[tokio::test]
async fn a_missing_directory_fails_with_the_machine_s_own_message() {
let dir = tree();
server/src/machines.rs
+21 -52
View File
@@ -127,7 +127,7 @@ impl OfferedModel {
/// Only llama.cpp, because only a driver that runs its own agent loop can use
/// one -- the coding CLIs configure MCP themselves and a second answer here
/// would quietly disagree with theirs.
fn mcp_defaults(kind: DriverKind) -> Vec<crate::config::McpServerConfig> {
pub fn mcp_defaults(kind: DriverKind) -> Vec<crate::config::McpServerConfig> {
match kind {
DriverKind::LlamaCpp => vec![crate::config::McpServerConfig {
name: "exa".to_string(),
@@ -270,41 +270,19 @@ pub fn id_from(label: &str) -> String {
}
}
/// Normalises what a phone keyboard produced: trims, drops blanks, and
/// expands a leading `~` the way a shell would.
/// Normalises what a phone keyboard produced: trims, and reads a field left
/// blank as absent rather than as an empty answer.
///
/// It deliberately does **not** touch a leading `~`. A path is stored as it was
/// typed, because `~` and `/home/someone` are not two spellings of one path --
/// the second is a snapshot of where the first pointed, and it is the snapshot
/// that breaks when an account is renamed or the value is read on another
/// machine. Expansion belongs where the path is used, against the machine it
/// belongs to: `ssh::quote_path` and `files::PATH_PRELUDE` for a remote one,
/// `ssh::expand_home` for one on this machine.
pub fn tidy(value: &str) -> Option<String> {
let value = value.trim();
if value.is_empty() {
return None;
}
Some(match value.strip_prefix("~/") {
Some(rest) => match std::env::home_dir() {
Some(home) => home.join(rest).to_string_lossy().into_owned(),
None => value.to_string(),
},
None => value.to_string(),
})
}
/// The inverse of [`tidy`]'s expansion: an absolute path under this machine's
/// home, written back as `~/…`, so that a working directory reads on a phone the
/// way it is written by hand.
///
/// Applied only to paths on **this** machine. `$HOME` here says nothing about
/// the home directory of a machine reached over ssh, so a remote path is stored
/// exactly as it was typed and the remote shell is what expands it.
pub fn shorten_home(path: &str) -> String {
let Some(home) = std::env::home_dir() else {
return path.to_string();
};
let home = home.to_string_lossy();
// The separator has to be part of the match, or `/home/bobby` would be read
// as a path inside `/home/bob`.
match path.strip_prefix(home.as_ref()) {
Some("") => "~".to_string(),
Some(rest) if rest.starts_with('/') => format!("~{rest}"),
_ => path.to_string(),
}
(!value.is_empty()).then(|| value.to_string())
}
/// Runs a launch to completion and returns its stdout as text.
@@ -326,25 +304,16 @@ impl Transport {
mod tests {
use super::*;
/// The two halves of a home-relative path, which have to be inverses: what
/// is stored is what the phone draws, and what the phone sends back is what
/// a process is started in.
/// A path survives the boundary unchanged, tilde included -- the one thing
/// `tidy` must not do is decide where `~` is.
#[test]
fn a_home_path_shortens_and_expands_back() {
let Some(home) = std::env::home_dir() else {
return;
};
let full = home.join("repos/ai-app-2");
let full = full.to_string_lossy();
assert_eq!(shorten_home(&full), "~/repos/ai-app-2");
assert_eq!(shorten_home(&home.to_string_lossy()), "~");
assert_eq!(tidy("~/repos/ai-app-2").as_deref(), Some(full.as_ref()));
// Not a prefix match on the characters: a sibling directory whose name
// merely starts with the home directory's is not inside it.
let sibling = format!("{}-backup/notes", home.to_string_lossy());
assert_eq!(shorten_home(&sibling), sibling);
assert_eq!(shorten_home("/etc/hosts"), "/etc/hosts");
fn a_typed_path_is_stored_as_typed() {
assert_eq!(
tidy(" ~/repos/ai-app-2 ").as_deref(),
Some("~/repos/ai-app-2")
);
assert_eq!(tidy("/etc/hosts").as_deref(), Some("/etc/hosts"));
assert_eq!(tidy(" "), None);
}
#[test]
server/src/routes.rs
+50 -40
View File
@@ -418,40 +418,27 @@ struct SshRequest {
}
impl SshRequest {
/// Tidied at the boundary rather than stored as typed -- this came from a
/// phone keyboard, so it may have a stray space or a `~`.
/// Tidied at the boundary -- this came from a phone keyboard, so a field
/// may carry a stray space or have been left blank. Nothing here rewrites
/// a path: a `~` is stored as typed and expanded where it is used, by the
/// machine it belongs to. See [`crate::machines::tidy`].
fn into_config(self) -> Result<crate::config::SshConfig, ApiError> {
let address = crate::machines::tidy(&self.address)
let tidy = crate::machines::tidy;
let address = tidy(&self.address)
.ok_or_else(|| ApiError::BadRequest("a machine needs an address".to_string()))?;
let path = |value: Option<String>| {
value
.as_deref()
.and_then(tidy)
.map(std::path::PathBuf::from)
};
Ok(crate::config::SshConfig {
address,
port: self.port,
identity_file: self
.identity_file
.as_deref()
.and_then(crate::machines::tidy)
.map(std::path::PathBuf::from),
options: self
.options
.iter()
.filter_map(|o| crate::machines::tidy(o))
.collect(),
// Not `tidy`: that expands `~` to *this* machine's home, and this
// path is on the other one. The remote shell expands it there.
attachments_dir: self
.attachments_dir
.as_deref()
.map(str::trim)
.filter(|dir| !dir.is_empty())
.map(std::path::PathBuf::from),
// The same rule, and for the same reason: this directory is
// on the other machine, so a `~` in it is that machine's home.
models_dir: self
.models_dir
.as_deref()
.map(str::trim)
.filter(|dir| !dir.is_empty())
.map(std::path::PathBuf::from),
identity_file: path(self.identity_file),
options: self.options.iter().filter_map(|o| tidy(o)).collect(),
attachments_dir: path(self.attachments_dir),
models_dir: path(self.models_dir),
})
}
}
@@ -1373,6 +1360,33 @@ async fn spawn(manager: &Arc<SessionManager>, body: SpawnRequest) -> Result<Sess
None => None,
};
// The same question `set_cwd` asks, and for the same reason: a typed
// directory that is not there becomes a session whose process cannot start,
// reported long after the typo and pointing at nothing. Stored as typed --
// a `~` is the serving machine's home and stays one. Blank normalised to
// absent for the reason the title below is: a form whose directory field
// was left alone sends `""`, which is `Some`.
let typed_cwd = match body
.cwd
.as_ref()
.map(|cwd| cwd.to_string_lossy())
.filter(|cwd| !cwd.trim().is_empty())
{
Some(cwd) => {
let cwd = crate::files::check_path(&cwd).map_err(bad_request)?;
let machine = machine_by_id(manager, &body.machine)?;
let transport = crate::session::transport::Transport::for_machine(&machine);
if !crate::session::import::directory_exists(&transport, &cwd).await {
return Err(ApiError::BadRequest(format!(
"{} has no directory {cwd}",
machine.name
)));
}
Some(PathBuf::from(cwd))
}
None => None,
};
let spec = SpawnSpec {
machine: body.machine,
provider: body.provider,
@@ -1391,7 +1405,7 @@ async fn spawn(manager: &Arc<SessionManager>, body: SpawnRequest) -> Result<Sess
}),
model: body.model,
// Resumed where it was working, so the CLI picks up the same tree.
cwd: body.cwd.or_else(|| {
cwd: typed_cwd.or_else(|| {
seed.as_ref()
.map(|(chosen, _)| PathBuf::from(&chosen.cwd))
.filter(|cwd| cwd.as_os_str() != "")
@@ -1826,17 +1840,13 @@ async fn set_cwd(
machine.name
)));
}
// Stored in the short form, so the one path kept is the one the phone will
// draw -- rather than storing `/home/bob/…` and abbreviating it again at
// each place it is shown, which is two representations of one directory.
// Only where the machine runs here; see `machines::shorten_home`.
let stored = if machine.ssh.is_none() {
crate::machines::shorten_home(&cwd)
} else {
cwd.clone()
};
// Stored exactly as it was typed, `~` included. Rewriting it either way is
// wrong for the same reason: `~` and `/home/someone` are not the same path,
// they are one path and one snapshot of where it was, and the snapshot is
// what breaks when an account is renamed. Every place this is used expands
// it against the machine it belongs to.
manager
.set_session_cwd(&id, PathBuf::from(&stored))
.set_session_cwd(&id, PathBuf::from(&cwd))
.map_err(bad_request)?;
Ok(StatusCode::NO_CONTENT)
}
server/src/session/llama/mod.rs
+23 -7
View File
@@ -378,9 +378,11 @@ struct Shared {
/// are changeable while the session runs: they ride on the next request,
/// so unlike the server's own flags there is nothing to reload.
sampling: Mutex<serde_json::Map<String, Value>>,
/// The session's working directory, which is where its tools act. `None`
/// leaves that to `llama-server`, which is the honest answer rather than a
/// guess at one.
/// The session's working directory, which is where its tools act, resolved
/// on the machine serving this session. Absolute, because it is handed to
/// `llama-server` as a header it `chdir`s to with no shell to expand a
/// `~`. `None` leaves the directory to `llama-server`, which is the honest
/// answer rather than a guess at one.
cwd: Option<String>,
/// Set by [`Driver::interrupt`]; the streaming loop and the tool loop both
/// check it, leaving what was produced in the transcript.
@@ -474,16 +476,30 @@ impl LlamaDriver {
let sampling = sampling_from(&meta.params);
// Resolved on the machine that will serve this session, because it is
// the one path here that reaches something with no shell in front of
// it: `llama-server` `chdir`s to the header it is given. See
// `files::resolve_blocking`. A failure is the launch's, for the same
// reason a CLI session whose directory has gone fails at its own `cd`
// -- tools running somewhere other than where they were told to is
// worse than not starting.
let cwd = meta
.cwd
.as_ref()
.map(|path| {
crate::files::resolve_blocking(transport, &path.to_string_lossy()).with_context(
|| format!("this session's working directory, {}", path.display()),
)
})
.transpose()?;
let driver = Self {
shared: Arc::new(Shared {
sink,
transcript: transcript.to_path_buf(),
session_dir: session_dir.to_path_buf(),
sampling: Mutex::new(sampling),
cwd: meta
.cwd
.as_ref()
.map(|path| path.to_string_lossy().into_owned()),
cwd,
cancel: AtomicBool::new(false),
turns: Mutex::new(Turns::default()),
serving: Mutex::new(Serving::Loading),
server/src/session/llama/tools.rs
+6
View File
@@ -172,6 +172,12 @@ impl Tools {
/// say they use one. A session with no working directory sends none, and
/// `llama-server` falls back to its own -- which is the honest outcome:
/// this server has no better answer for where "here" is.
///
/// It must already be **absolute**: the header is passed to `chdir` with
/// no shell in the way, so a leading `~` is a directory of that name and
/// every tool using one fails with "failed to spawn process". The driver
/// resolves it on the serving machine at launch; see
/// `files::resolve_blocking`.
pub fn execute(
&self,
name: &str,
server/src/ssh.rs
+5 -1
View File
@@ -107,7 +107,11 @@ pub fn command(
command.args(["-p", &port.to_string()]);
}
if let Some(identity) = &ssh.identity_file {
command.arg("-i").arg(identity);
// Expanded here rather than when it was typed: this one *is* a path on
// this machine, but storing what `~` resolved to on the day it was
// entered is what breaks when the account is renamed. There is no shell
// between here and `ssh`, so nothing else would expand it.
command.arg("-i").arg(expand_home(identity));
// Without this, ssh may offer an agent key first and authenticate as
// somebody else entirely -- silently, and with different permissions.
command.args(["-o", "IdentitiesOnly=yes"]);