Two things, both about the transcript telling the truth about itself. Markdown is rendered rather than shown as its source. The parsing is mikepenz/multiplatform-markdown-renderer, not something written here: markdown is somebody else's specification, and a hand-written subset of one disagrees with it at the edges, which is where the bug reports come from. `Markdown.kt` is only the mapping onto this app's palette, so code, links and rules take the Catppuccin values the rest of the app uses rather than the renderer's defaults. The queued-message list was cleared wholesale whenever a turn ended. But the backend holds a queue of its own and takes one message per turn, so a turn ending is precisely the moment the *rest* are still waiting -- the bubbles vanished while the messages were on their way, which reads as everything after the first having been dropped. Now a held message leaves the list exactly two ways: the session reads it, which arrives as a UserMessage, or its send failed and there is nothing to wait for. Measured first, because the report was that the backend dropped them: three messages sent behind one long turn were all delivered in order (ONE, TWO, THREE) against current main, so the loss was in the display. Verified on the emulator: headings, emphasis, inline code, nested lists, a quote bar, a fenced block, a rule and a link all render, and the three queued messages sit through their turn. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
277 lines
11 KiB
Rust
277 lines
11 KiB
Rust
//! Building the command a driver actually spawns -- locally, or wrapped in
|
|
//! `ssh` when the session names a host to run on.
|
|
//!
|
|
//! The whole point of the session design is that a driver speaks JSONL over
|
|
//! a child process's stdio and doesn't care what that child is. A remote
|
|
//! session is therefore the identical command with `ssh host …` in front:
|
|
//! stdio doesn't care, so nothing downstream of here changes.
|
|
//!
|
|
//! Uses the system `ssh` client rather than a Rust SSH library, so
|
|
//! `~/.ssh/config`, agents, and jump hosts all keep working and there is
|
|
//! only one place to configure connections (PLAN.md, rule 23).
|
|
|
|
use std::path::Path;
|
|
use std::process::Command;
|
|
|
|
use crate::config::SshConfig;
|
|
|
|
/// Options forced onto every connection. `BatchMode` makes a missing key
|
|
/// fail immediately with a readable message instead of hanging on a
|
|
/// password prompt that nothing can answer; the keepalives turn a silently
|
|
/// dropped link into a process exit, which the session reports as `exited`
|
|
/// rather than appearing to hang forever.
|
|
const SSH_OPTIONS: [&str; 3] = [
|
|
"BatchMode=yes",
|
|
"ServerAliveInterval=30",
|
|
"ServerAliveCountMax=3",
|
|
];
|
|
|
|
/// Builds the child process for `program args…`, run in `cwd`, either on
|
|
/// this machine (`ssh` absent) or on the machine it describes.
|
|
///
|
|
/// Stdio is left alone: how the streams are connected is the caller's
|
|
/// decision and differs by more than the transport does -- a probe wants
|
|
/// pipes it will drain, a session wants files that outlive this server --
|
|
/// so `Transport::spawn` applies it rather than this.
|
|
///
|
|
/// A plain [`std::process::Command`], which `tokio` converts from, because
|
|
/// not every caller is async: the usage fetch is blocking by nature (it
|
|
/// makes a blocking HTTP call) and reads a file from the same machine on
|
|
/// the way, and it should not have to build an ssh invocation of its own
|
|
/// to do that. One place knows what a correct invocation is; how it is run
|
|
/// is the caller's business.
|
|
pub fn command(
|
|
remote: Option<&SshConfig>,
|
|
program: &str,
|
|
args: &[String],
|
|
cwd: Option<&Path>,
|
|
) -> Command {
|
|
let Some(ssh) = remote else {
|
|
let mut command = Command::new(program);
|
|
command.args(args);
|
|
if let Some(cwd) = cwd {
|
|
command.current_dir(cwd);
|
|
}
|
|
return command;
|
|
};
|
|
|
|
let mut command = Command::new("ssh");
|
|
// -T: no pty. This carries JSONL, and a pty would rewrite it (echo,
|
|
// CRLF translation, ^C handling) into something the parser can't read.
|
|
command.arg("-T");
|
|
for option in SSH_OPTIONS {
|
|
command.args(["-o", option]);
|
|
}
|
|
for option in &ssh.options {
|
|
command.args(["-o", option]);
|
|
}
|
|
if let Some(port) = ssh.port {
|
|
command.args(["-p", &port.to_string()]);
|
|
}
|
|
if let Some(identity) = &ssh.identity_file {
|
|
command.arg("-i").arg(identity);
|
|
// Without this, ssh may offer an agent key first and authenticate
|
|
// as somebody else entirely -- silently, and with different
|
|
// permissions than intended.
|
|
command.args(["-o", "IdentitiesOnly=yes"]);
|
|
}
|
|
command.arg(&ssh.address);
|
|
command.arg(remote_script(program, args, cwd));
|
|
command
|
|
}
|
|
|
|
/// The single argument handed to the remote login shell.
|
|
///
|
|
/// `exec` so the CLI replaces that shell: the process the connection is
|
|
/// attached to is then the CLI itself, and dropping the connection takes
|
|
/// it down rather than leaving an orphan behind a live wrapper.
|
|
fn remote_script(program: &str, args: &[String], cwd: Option<&Path>) -> String {
|
|
let mut script = String::new();
|
|
if let Some(cwd) = cwd {
|
|
script.push_str("cd ");
|
|
script.push_str("e_path(&cwd.to_string_lossy()));
|
|
script.push_str(" && ");
|
|
}
|
|
script.push_str("exec ");
|
|
script.push_str("e(program));
|
|
for arg in args {
|
|
script.push(' ');
|
|
script.push_str("e(arg));
|
|
}
|
|
script
|
|
}
|
|
|
|
/// Quotes a path, expanding a leading `~` and nothing else.
|
|
///
|
|
/// [`quote`] is right for every other word crossing to the remote side and
|
|
/// wrong for exactly one character. `~` means "expand me", and single
|
|
/// quotes are what stop expansion -- so a working directory typed as
|
|
/// `~/repos/ai-app` arrived as the literal four-character directory `~`,
|
|
/// and the remote shell said it did not exist. Which is true, and reads
|
|
/// like the path being wrong rather than the quoting.
|
|
///
|
|
/// `"$HOME"` rather than handing the tilde to the shell unquoted: the
|
|
/// variable is expanded, the expansion is not re-split or globbed because
|
|
/// it is double-quoted, and everything after it stays single-quoted and
|
|
/// literal. So the one character that has to mean something keeps meaning
|
|
/// it, and nothing else gains a meaning. `$HOME` is set by every shell
|
|
/// this can land in, including the fish login shell on the dev VM, which
|
|
/// is why this does not depend on the remote shell being POSIX.
|
|
///
|
|
/// `~user` is deliberately not handled: there is no portable expansion for
|
|
/// it, and inventing one would mean guessing another account's home
|
|
/// directory. It stays literal and fails with the shell's own message.
|
|
fn quote_path(path: &str) -> String {
|
|
if path == "~" {
|
|
return "\"$HOME\"".to_string();
|
|
}
|
|
match path.strip_prefix("~/") {
|
|
Some(rest) => format!("\"$HOME\"/{}", quote(rest)),
|
|
None => quote(path),
|
|
}
|
|
}
|
|
|
|
/// Single-quotes one word for a POSIX shell.
|
|
///
|
|
/// Everything crossing to the remote side goes through here: paths, model
|
|
/// names, and prompts-as-arguments are all attacker-adjacent input in a
|
|
/// server whose whole job is running commands, and unquoted they would be
|
|
/// shell syntax rather than data.
|
|
fn quote(word: &str) -> String {
|
|
// Inside single quotes every character is literal except `'` itself,
|
|
// which is closed, escaped, and reopened.
|
|
format!("'{}'", word.replace('\'', r"'\''"))
|
|
}
|
|
|
|
#[cfg(test)]
|
|
mod tests {
|
|
use super::*;
|
|
|
|
fn args<const N: usize>(args: [&str; N]) -> Vec<String> {
|
|
args.iter().map(|arg| arg.to_string()).collect()
|
|
}
|
|
|
|
/// The rendered argv, for asserting on what would actually run.
|
|
fn argv(command: &Command) -> Vec<String> {
|
|
std::iter::once(command.get_program())
|
|
.chain(command.get_args())
|
|
.map(|arg| arg.to_string_lossy().into_owned())
|
|
.collect()
|
|
}
|
|
|
|
/// A host with nothing configured but a name to dial, so `~/.ssh/config`
|
|
/// decides everything else -- the case that proves this adds no flags of
|
|
/// its own when it was not told to.
|
|
fn bare_host() -> SshConfig {
|
|
SshConfig {
|
|
address: "vm".to_string(),
|
|
port: None,
|
|
identity_file: None,
|
|
options: vec![],
|
|
}
|
|
}
|
|
|
|
#[test]
|
|
fn a_session_with_no_host_runs_the_command_directly() {
|
|
let command = command(
|
|
None,
|
|
"claude",
|
|
&args(["-p", "--verbose"]),
|
|
Some(Path::new("/tmp/x")),
|
|
);
|
|
assert_eq!(argv(&command), ["claude", "-p", "--verbose"]);
|
|
assert_eq!(command.get_current_dir(), Some(Path::new("/tmp/x")));
|
|
}
|
|
|
|
#[test]
|
|
fn a_session_with_a_host_wraps_the_same_command_in_ssh() {
|
|
let ssh = SshConfig {
|
|
address: "bob@10.0.2.15".to_string(),
|
|
port: Some(2222),
|
|
identity_file: Some("/home/me/.ssh/id_ai".into()),
|
|
options: vec!["StrictHostKeyChecking=accept-new".to_string()],
|
|
};
|
|
let rendered = argv(&command(
|
|
Some(&ssh),
|
|
"claude",
|
|
&args(["-p", "--model", "haiku"]),
|
|
Some(Path::new("/home/bob/work")),
|
|
));
|
|
|
|
assert_eq!(rendered[0], "ssh");
|
|
assert!(rendered.contains(&"-T".to_string()));
|
|
assert!(rendered.contains(&"BatchMode=yes".to_string()));
|
|
assert!(rendered.contains(&"StrictHostKeyChecking=accept-new".to_string()));
|
|
assert!(rendered.contains(&"IdentitiesOnly=yes".to_string()));
|
|
assert!(rendered.contains(&"2222".to_string()));
|
|
assert!(rendered.contains(&"/home/me/.ssh/id_ai".to_string()));
|
|
// The host, then exactly one argument: the remote script.
|
|
assert_eq!(rendered[rendered.len() - 2], "bob@10.0.2.15");
|
|
assert_eq!(
|
|
rendered[rendered.len() - 1],
|
|
"cd '/home/bob/work' && exec 'claude' '-p' '--model' 'haiku'",
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn a_remote_command_without_a_cwd_just_execs() {
|
|
let ssh = bare_host();
|
|
let rendered = argv(&command(Some(&ssh), "claude", &args(["-p"]), None));
|
|
assert_eq!(rendered.last().unwrap(), "exec 'claude' '-p'");
|
|
// No -i means no IdentitiesOnly: ~/.ssh/config decides instead.
|
|
assert!(!rendered.contains(&"IdentitiesOnly=yes".to_string()));
|
|
}
|
|
|
|
/// The one character quoting must not swallow.
|
|
///
|
|
/// A working directory typed as `~/repos/ai-app` was arriving as the
|
|
/// literal directory `~`, and the remote shell reported it missing --
|
|
/// which reads as the path being wrong rather than the quoting being
|
|
/// wrong, and cost an evening on exactly that misreading.
|
|
#[test]
|
|
fn a_leading_tilde_expands_and_nothing_else_does() {
|
|
assert_eq!(quote_path("~"), "\"$HOME\"");
|
|
assert_eq!(quote_path("~/repos/ai-app"), "\"$HOME\"/'repos/ai-app'");
|
|
// Only leading, and only its own segment: a tilde anywhere else is
|
|
// an ordinary character in a filename, and `~user` has no portable
|
|
// expansion so it stays literal and fails with the shell's message.
|
|
assert_eq!(quote_path("/tmp/~/x"), "'/tmp/~/x'");
|
|
assert_eq!(quote_path("~user/x"), "'~user/x'");
|
|
|
|
// And it reaches the script the remote shell is handed.
|
|
assert_eq!(
|
|
remote_script("claude", &args(["-p"]), Some(Path::new("~/repos/ai-app"))),
|
|
"cd \"$HOME\"/'repos/ai-app' && exec 'claude' '-p'",
|
|
);
|
|
}
|
|
|
|
#[test]
|
|
fn shell_metacharacters_cross_as_data_not_syntax() {
|
|
// Expanding $HOME must not open a door for anything else: the rest
|
|
// stays single-quoted, so this remains one absurd path rather than
|
|
// three commands.
|
|
assert_eq!(
|
|
quote_path("~/'; touch /tmp/pwned; '"),
|
|
r#""$HOME"/''\''; touch /tmp/pwned; '\'''"#,
|
|
);
|
|
|
|
assert_eq!(quote("plain"), "'plain'");
|
|
assert_eq!(quote("with space"), "'with space'");
|
|
assert_eq!(quote("; rm -rf /"), "'; rm -rf /'");
|
|
assert_eq!(quote("$(whoami)"), "'$(whoami)'");
|
|
assert_eq!(quote("it's"), r"'it'\''s'");
|
|
|
|
// The end-to-end version of the same worry: a working directory
|
|
// that tries to close the quote and start a new command.
|
|
let ssh = bare_host();
|
|
let evil = Path::new("/tmp/'; touch /tmp/pwned; '");
|
|
let rendered = argv(&command(Some(&ssh), "claude", &[], Some(evil)));
|
|
let script = rendered.last().unwrap();
|
|
assert_eq!(
|
|
script,
|
|
r"cd '/tmp/'\''; touch /tmp/pwned; '\''' && exec 'claude'"
|
|
);
|
|
assert!(!script.contains("; touch /tmp/pwned; '\" "));
|
|
}
|
|
}
|