diff --git a/docs/next/website/src/content/docs/ja/troubleshooting.mdx b/docs/next/website/src/content/docs/ja/troubleshooting.mdx index e5aa64c9ea..bedf42b3d3 100644 --- a/docs/next/website/src/content/docs/ja/troubleshooting.mdx +++ b/docs/next/website/src/content/docs/ja/troubleshooting.mdx @@ -87,6 +87,29 @@ herdr サーバーを停止するとペインのプロセスも終了します。Herdr のペインは長時間動作するサーバーの macOS 起動コンテキストを継承するため、SSH やバックグラウンドジョブから起動されたサーバーは対話型 Keychain サービスにアクセスできないことがあります。詳細は [Herdr issue #966](https://github.com/herdrdev/herdr/issues/966) を参照してください。 +## ペインにコマンドではなく `. /…/staged-commands/…` が表示される + +フォアグラウンドのプログラムがカノニカルモードで読み取っている間、ターミナルが 1 +行で受け取れるバイト数には上限があります (macOS では 1024、Linux では 4096)。 +これを超えるとカーネルは行の残りを黙って破棄するため、長いコマンドは切り詰められ +たまま送られ、実行されません。 + +そのため `herdr agent start` は 512 バイトを超えるコマンドをプライベートなファイル +に書き出し、それを実行する短い行を入力します。ペインに表示されるのがこの行です。 +ファイルは実行された時点で削除されます。これより短いコマンドはそのまま入力される +ため、`--append-system-prompt` のような長い引数のときだけこの表示になります。 + +この方式が使われるのは、Herdr が読み込み方を把握しているシェルだけです。`sh`、 +`bash`、`dash`、`zsh`、`ksh`、`mksh` は `.` を、`fish` は `source` を使います。 +`csh`、`tcsh`、`nu`、`elvish`、`xonsh` と Windows では、コマンドはそのまま入力され、 +行長制限を受けたままになります。 + +`herdr pane send-text`、`herdr pane run`、ペインへの貼り付けで送ったテキストは +そのまま文字として送られるため、引き続きターミナルの行長制限を受けます。シェルは +見た目以上に頻繁にカノニカルモードになります。`bash` と `zsh` は実行するコマンド +の間ずっとカノニカルモードに戻り、Ubuntu の `/bin/sh` である `dash` は一度も抜け +ません。長いテキストはファイルに書き出し、そのファイルを実行してください。 + ## `herdr` コマンドが見つからない ターミナルを再起動して環境を読み込み直し、Herdr のインストール先が `PATH` に含まれていることを確認してください。パッケージマネージャー経由のインストールは、そのパッケージマネージャーから更新して公開する必要があります。[インストール](/ja/docs/install/#verify)を参照してください。 diff --git a/docs/next/website/src/content/docs/troubleshooting.mdx b/docs/next/website/src/content/docs/troubleshooting.mdx index 69e6511047..3c95332fdc 100644 --- a/docs/next/website/src/content/docs/troubleshooting.mdx +++ b/docs/next/website/src/content/docs/troubleshooting.mdx @@ -87,6 +87,30 @@ herdr Stopping the server exits its pane processes. Herdr panes inherit the long-lived server's macOS launch context, so a server started through SSH or a background job may not have access to interactive Keychain services. See [Herdr issue #966](https://github.com/herdrdev/herdr/issues/966) for details. +## A pane shows `. /…/staged-commands/…` instead of the command + +A terminal accepts only a limited number of bytes on a single input line while +the foreground program is reading in canonical mode — 1024 on macOS, 4096 on +Linux. Beyond that the kernel silently discards the rest of the line, so a long +command would be delivered truncated and never run. + +`herdr agent start` therefore writes a command longer than 512 bytes to a +private file and types a short line that runs it, which is what you see in the +pane. The file is removed the moment it runs. Commands shorter than that are +typed unchanged, so this only appears for long arguments such as +`--append-system-prompt`. + +Only shells Herdr knows how to source in are staged this way: `sh`, `bash`, +`dash`, `zsh`, `ksh` and `mksh` use `.`, and `fish` uses `source`. In `csh`, +`tcsh`, `nu`, `elvish` and `xonsh`, and on Windows, the command is typed +unchanged and remains subject to the limit. + +Text sent with `herdr pane send-text`, `herdr pane run`, or pasted into a pane +is delivered literally and is still subject to the terminal's line limit. Shells +are in canonical mode more often than they look: `bash` and `zsh` return to it +for the duration of every command they run, and `dash` — `/bin/sh` on Ubuntu — +never leaves it. For long text, write it to a file and run that file instead. + ## The `herdr` command is not found Restart the terminal so it reloads its environment, then confirm the Herdr install directory is on `PATH`. For package-manager installs, use that package manager to update Herdr and expose it on `PATH`. See [Install Herdr](/docs/install/#verify). diff --git a/docs/next/website/src/content/docs/zh-cn/troubleshooting.mdx b/docs/next/website/src/content/docs/zh-cn/troubleshooting.mdx index 7aa864fb9d..ca8e8ce9ba 100644 --- a/docs/next/website/src/content/docs/zh-cn/troubleshooting.mdx +++ b/docs/next/website/src/content/docs/zh-cn/troubleshooting.mdx @@ -87,6 +87,25 @@ herdr 停止服务器会结束窗格进程。Herdr 窗格会继承长期运行服务器的 macOS 启动上下文,因此通过 SSH 或后台任务启动的服务器可能无法访问交互式钥匙串服务。详情见 [Herdr issue #966](https://github.com/herdrdev/herdr/issues/966)。 +## 窗格中显示 `. /…/staged-commands/…` 而不是命令 + +当前台程序以规范模式 (canonical mode) 读取输入时,终端单行只能接受有限的字节数: +macOS 为 1024,Linux 为 4096。超出部分会被内核静默丢弃,因此过长的命令只会被截断 +送达,而不会执行。 + +为此,`herdr agent start` 会把超过 512 字节的命令写入一个私有文件,并输入一行短命 +令来运行它,这就是你在窗格中看到的内容。该文件在运行的那一刻即被删除。更短的命令 +会原样输入,所以只有 `--append-system-prompt` 这类长参数才会出现这种显示。 + +只有 Herdr 知道如何加载的 shell 才会这样暂存:`sh`、`bash`、`dash`、`zsh`、`ksh` +和 `mksh` 使用 `.`,`fish` 使用 `source`。在 `csh`、`tcsh`、`nu`、`elvish`、`xonsh` +以及 Windows 上,命令会原样输入,并且仍受行长限制。 + +通过 `herdr pane send-text`、`herdr pane run` 或粘贴发送到窗格的文本仍按字面逐字送 +达,因此依旧受终端行长限制。shell 处于规范模式的时间比看上去更多:`bash` 和 `zsh` +在其运行的每条命令期间都会回到规范模式,而 Ubuntu 上作为 `/bin/sh` 的 `dash` 从不 +离开规范模式。文本较长时,请先写入文件再运行该文件。 + ## 找不到 `herdr` 命令 重启终端以重新加载环境,然后确认 Herdr 安装目录位于 `PATH` 中。通过软件包管理器安装的 Herdr 必须通过该管理器更新并加入环境。见[安装 Herdr](/zh-cn/docs/install/#verify)。 diff --git a/src/app/agents.rs b/src/app/agents.rs index f25383fe93..7eaf3eb182 100644 --- a/src/app/agents.rs +++ b/src/app/agents.rs @@ -196,24 +196,34 @@ impl App { argv.extend(params.args); let command = crate::platform::interactive_shell_command(&argv, &shell_name) .ok_or(AgentStartError::InvalidArgument)?; - let bytes = crate::app::api_helpers::encode_api_submission(runtime, &command); let timeout = Duration::from_millis( params .timeout_ms .unwrap_or(DEFAULT_AGENT_START_TIMEOUT.as_millis() as u64), ); + // Reject before staging, so no rejected request leaves a payload behind. if timeout <= AGENT_START_SETTLE_DELAY || timeout > MAX_AGENT_START_TIMEOUT { return Err(AgentStartError::InvalidTimeout); } + // A composed agent command can carry a system prompt far longer than a + // terminal will accept on one line, so stage it rather than type it + // (refs #2862). + let staged = crate::app::command_delivery::stage( + &command, + &shell_name, + &format!("agent-{}", params.kind), + ) + .map_err(|err| AgentStartError::InputFailed(err.to_string()))?; + let bytes = crate::app::api_helpers::encode_api_submission(runtime, &staged.text); let now = Instant::now(); - let terminal = self - .state - .terminals - .get_mut(&terminal_id) - .ok_or_else(|| AgentStartError::TargetUnavailable(params.pane_id.clone()))?; + let Some(terminal) = self.state.terminals.get_mut(&terminal_id) else { + staged.discard(); + return Err(AgentStartError::TargetUnavailable(params.pane_id.clone())); + }; terminal.begin_managed_agent(name.clone(), kind, now, AGENT_START_SETTLE_DELAY, timeout); if let Err(err) = runtime.try_send_bytes(Bytes::from(bytes)) { + staged.discard(); terminal.clear_agent_name(); return Err(AgentStartError::InputFailed(err.to_string())); } diff --git a/src/app/command_delivery.rs b/src/app/command_delivery.rs new file mode 100644 index 0000000000..09f7ed927c --- /dev/null +++ b/src/app/command_delivery.rs @@ -0,0 +1,376 @@ +//! Delivering a composed command into a pane without typing an over-long line. +//! +//! A terminal in canonical mode assembles one input line in a fixed-size buffer +//! and hands the reader nothing until a terminator arrives, so a longer line is +//! never delivered: the kernel keeps a prefix, discards the rest, and still +//! reports the whole `write()` as successful (refs #2862). +//! +//! Canonical mode is the default, not an edge case. `bash` and `zsh` leave it +//! only while their line editor reads, and `dash` -- `/bin/sh` on Ubuntu -- +//! never leaves it at all, so herdr can neither wait the condition out nor +//! detect its way around it. An over-long command is written to a file and +//! sourced instead, which makes the typed line a function of the path length +//! rather than the payload's. + +use std::io; +#[cfg(unix)] +use std::path::Path; +use std::path::PathBuf; + +/// The longest command herdr types directly into a pane. +/// +/// The canonical buffer is 1024 bytes on Darwin and 4096 on Linux, both +/// measured exactly against a bare PTY pair. This sits under the smaller with +/// room to spare, and well above any ordinary command, so the file path is +/// reached only by payloads that would otherwise be truncated. +/// +/// It bounds what herdr itself types; it cannot bound what is already sitting +/// unread in the terminal's buffer, which no threshold can. +#[cfg(unix)] +const MAX_TYPED_COMMAND: usize = 512; + +pub(crate) struct StagedCommand { + pub(crate) text: String, + /// The payload file backing `text`, when the command was too long to type. + payload: Option, +} + +impl StagedCommand { + fn typed(command: &str) -> Self { + Self { + text: command.to_string(), + payload: None, + } + } + + /// Remove the payload when the command could not be handed to the pane, so + /// a failed send does not leave it staged on disk. A delivered command + /// removes its own file as the first thing it does. + pub(crate) fn discard(self) { + if let Some(path) = self.payload { + let _ = std::fs::remove_file(path); + } + } +} + +/// Stage `command` for a pane whose shell is `shell_name`, labelling any +/// payload file with `label` so the line the user sees names what it runs. +#[cfg(unix)] +pub(crate) fn stage(command: &str, shell_name: &str, label: &str) -> io::Result { + if command.len() <= MAX_TYPED_COMMAND { + return Ok(StagedCommand::typed(command)); + } + stage_in(&payload_dir(), command, shell_name, label) +} + +/// ConPTY has no canonical line discipline, so Windows types every command. +#[cfg(windows)] +pub(crate) fn stage(command: &str, _shell_name: &str, _label: &str) -> io::Result { + Ok(StagedCommand::typed(command)) +} + +#[cfg(unix)] +fn payload_dir() -> PathBuf { + crate::config::state_dir().join("staged-commands") +} + +#[cfg(unix)] +fn stage_in(dir: &Path, command: &str, shell_name: &str, label: &str) -> io::Result { + use std::os::unix::fs::{OpenOptionsExt, PermissionsExt}; + + let Some(keyword) = crate::platform::shell_source_keyword(shell_name) else { + // Staging would hand this shell a line it cannot run, which is worse + // than the truncation it replaces: a command under the line cap works + // today. Leave it typed and say so. + tracing::warn!( + shell = %shell_name, + bytes = command.len(), + "typing a command that may exceed the terminal line limit: herdr cannot source a file in this shell" + ); + return Ok(StagedCommand::typed(command)); + }; + + std::fs::create_dir_all(dir)?; + std::fs::set_permissions(dir, std::fs::Permissions::from_mode(0o700))?; + + let path = dir.join(payload_name(label)); + let path_arg = shell_path(&path)?; + let remove = shell_command(&["rm", "-f", "--", &path_arg], shell_name)?; + + // Opened before the cleanup below can apply: `create_new` means a failure + // here found someone else's file, which is not ours to remove. + let mut file = std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .mode(0o600) + .open(&path)?; + + // Every failure past this point has to take the file with it. `discard` + // only runs on a `StagedCommand`, and these paths never return one. + match fill_payload(&mut file, &remove, command, keyword, &path_arg, shell_name) { + Ok(text) => Ok(StagedCommand { + text, + payload: Some(path), + }), + Err(err) => { + let _ = std::fs::remove_file(&path); + Err(err) + } + } +} + +/// Write the payload into `file` and render the line that sources it. +#[cfg(unix)] +fn fill_payload( + file: &mut std::fs::File, + remove: &str, + command: &str, + keyword: &str, + path_arg: &str, + shell_name: &str, +) -> io::Result { + use std::io::Write; + + // The removal runs first: unlinking only drops the directory entry, and the + // shell keeps reading through its open descriptor, so the command cleans + // itself up immediately instead of outliving a long-running agent. + writeln!(file, "{remove}")?; + writeln!(file, "{command}")?; + file.sync_all()?; + + let text = shell_command(&[keyword, path_arg], shell_name)?; + // The typed line is bounded by the path, not the payload, but a state + // directory near PATH_MAX could still overrun the cap. Fail loudly rather + // than type a line the line discipline will silently cut. + if text.len() > MAX_TYPED_COMMAND { + return Err(io::Error::new( + io::ErrorKind::InvalidData, + "staged command path is too long to source in one line", + )); + } + Ok(text) +} + +#[cfg(unix)] +fn shell_path(path: &Path) -> io::Result { + path.to_str().map(str::to_string).ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidData, + "staged command path is not valid UTF-8", + ) + }) +} + +#[cfg(unix)] +fn shell_command(argv: &[&str], shell_name: &str) -> io::Result { + let argv: Vec = argv.iter().map(|part| (*part).to_string()).collect(); + crate::platform::interactive_shell_command(&argv, shell_name).ok_or_else(|| { + io::Error::new( + io::ErrorKind::InvalidData, + "could not quote a staged command for this shell", + ) + }) +} + +/// A unique, readable file name. The label is what the user sees in the pane, +/// so it is kept intact apart from characters that do not belong in a path. +#[cfg(unix)] +fn payload_name(label: &str) -> String { + let label: String = label + .chars() + .map(|ch| { + if ch.is_ascii_alphanumeric() || ch == '-' { + ch + } else { + '-' + } + }) + .collect(); + let nanos = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|elapsed| elapsed.as_nanos()) + .unwrap_or_default(); + format!("{label}-{}-{nanos:x}", std::process::id()) +} + +#[cfg(all(test, unix))] +mod tests { + use super::*; + use std::os::unix::fs::PermissionsExt; + + fn scratch(label: &str) -> PathBuf { + let nanos = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .map(|elapsed| elapsed.as_nanos()) + .unwrap_or_default(); + let dir = std::env::temp_dir().join(format!( + "herdr-staged-{label}-{}-{nanos:x}", + std::process::id() + )); + let _ = std::fs::remove_dir_all(&dir); + dir + } + + /// refs #2862: an ordinary command is typed exactly as it always was, so + /// the common path gains no file, no syscall and no new failure mode. + #[test] + fn an_ordinary_command_is_typed_unchanged() { + let dir = scratch("ordinary"); + let staged = stage("claude --model sonnet", "zsh", "agent-claude").expect("stage"); + assert_eq!(staged.text, "claude --model sonnet"); + assert!(staged.payload.is_none()); + assert!(!dir.exists(), "no payload directory should be created"); + } + + #[test] + fn an_over_long_command_is_staged_to_a_file() { + let dir = scratch("over-long"); + let command = format!("claude --append-system-prompt '{}'", "A".repeat(4000)); + let staged = stage_in(&dir, &command, "zsh", "agent-claude").expect("stage"); + + let path = staged.payload.clone().expect("payload path"); + assert!( + staged.text.len() < MAX_TYPED_COMMAND, + "the typed line must be short, got {}: {}", + staged.text.len(), + staged.text + ); + assert!(staged.text.starts_with(". "), "typed: {}", staged.text); + assert!( + staged.text.contains("agent-claude"), + "typed: {}", + staged.text + ); + + let body = std::fs::read_to_string(&path).expect("payload readable"); + assert!( + body.lines() + .next() + .expect("first line") + .starts_with("rm -f --"), + "the payload must remove itself first: {body:?}" + ); + assert!( + body.contains(&command), + "the payload must carry the command" + ); + + let mode = std::fs::metadata(&path) + .expect("metadata") + .permissions() + .mode(); + assert_eq!(mode & 0o777, 0o600, "payload must not be world readable"); + let dir_mode = std::fs::metadata(&dir) + .expect("dir metadata") + .permissions() + .mode(); + assert_eq!(dir_mode & 0o777, 0o700); + + staged.discard(); + assert!(!path.exists(), "discard must remove an undelivered payload"); + let _ = std::fs::remove_dir_all(&dir); + } + + /// refs #2862: the typed line is a function of the path, not the payload, + /// which is the property that takes the line cap out of play. + #[test] + fn the_typed_line_does_not_grow_with_the_command() { + let dir = scratch("length"); + let small = stage_in(&dir, &format!("claude '{}'", "A".repeat(1_000)), "zsh", "a") + .expect("stage small"); + let large = stage_in( + &dir, + &format!("claude '{}'", "A".repeat(900_000)), + "zsh", + "a", + ) + .expect("stage large"); + assert_eq!( + small.text.len(), + large.text.len(), + "a 1 KB and a 900 KB command must type the same number of bytes" + ); + small.discard(); + large.discard(); + let _ = std::fs::remove_dir_all(&dir); + } + + /// refs #2862: a shell herdr cannot source in keeps the command it would + /// have typed before. Rewriting it into a line that shell cannot run would + /// break commands under the cap that work today. + #[test] + fn a_shell_with_unknown_source_syntax_is_never_rewritten() { + let dir = scratch("unknown-shell"); + let command = format!("claude '{}'", "A".repeat(4000)); + for shell in ["nu", "csh", "tcsh", "elvish", "xonsh"] { + let staged = stage_in(&dir, &command, shell, "agent-claude").expect("stage"); + assert_eq!(staged.text, command, "{shell} must be typed unchanged"); + assert!(staged.payload.is_none(), "{shell} must stage no payload"); + } + assert!(!dir.exists(), "no payload directory should be created"); + } + + /// A pane reports its shell as the process gives it: a login shell as + /// `-zsh`, another as a full path. Both must still be recognised, or + /// staging would quietly stop happening for the shells that need it most. + #[test] + fn a_shell_name_is_normalised_before_its_keyword_is_chosen() { + let dir = scratch("normalised"); + let command = format!("claude '{}'", "A".repeat(4000)); + for shell in ["-zsh", "/bin/ksh"] { + let staged = stage_in(&dir, &command, shell, "agent-claude").expect("stage"); + assert!( + staged.text.starts_with(". "), + "{shell} must dot-source: {}", + staged.text + ); + staged.discard(); + } + let _ = std::fs::remove_dir_all(&dir); + } + + /// A path long enough to push the source line over the cap must fail + /// loudly and leave nothing behind: refusing is the point, and a payload + /// that outlives its refusal is the leak this module keeps closing. + #[test] + fn a_path_too_long_to_source_refuses_and_stages_nothing() { + // Two components under the 255-byte filename limit, long enough that + // `. ''` cannot fit in MAX_TYPED_COMMAND. + let dir = scratch("long-path") + .join("d".repeat(230)) + .join("e".repeat(230)); + let command = format!("claude '{}'", "A".repeat(4000)); + + let err = match stage_in(&dir, &command, "zsh", "agent-claude") { + Ok(_) => panic!("staging must refuse a path too long to source"), + Err(err) => err, + }; + assert_eq!(err.kind(), io::ErrorKind::InvalidData, "{err}"); + + let leftover: Vec<_> = std::fs::read_dir(&dir) + .expect("directory exists") + .flatten() + .map(|entry| entry.path()) + .collect(); + assert!( + leftover.is_empty(), + "a refused staging must leave no payload: {leftover:?}" + ); + let _ = std::fs::remove_dir_all(scratch("long-path")); + } + + /// fish dropped `.` as a source alias, so it needs the spelled-out form. + #[test] + fn fish_sources_with_its_own_keyword() { + let dir = scratch("fish"); + let command = format!("claude '{}'", "A".repeat(4000)); + let staged = stage_in(&dir, &command, "fish", "agent-claude").expect("stage"); + assert!( + staged.text.starts_with("source "), + "fish must not be handed `.`: {}", + staged.text + ); + staged.discard(); + let _ = std::fs::remove_dir_all(&dir); + } +} diff --git a/src/app/mod.rs b/src/app/mod.rs index 131b9729af..494aad508f 100644 --- a/src/app/mod.rs +++ b/src/app/mod.rs @@ -10,6 +10,7 @@ mod agents; pub(crate) use agents::{AGENT_START_SETTLE_DELAY, MAX_AGENT_START_TIMEOUT}; mod api; mod api_helpers; +mod command_delivery; pub(crate) use api_helpers::limit_snapshot_lines; mod creation; mod custom_commands; diff --git a/src/platform/mod.rs b/src/platform/mod.rs index f15c71efca..ce65f2c3fb 100644 --- a/src/platform/mod.rs +++ b/src/platform/mod.rs @@ -325,6 +325,22 @@ pub(crate) fn quote_powershell_arg(value: &str) -> String { format!("'{}'", value.replace('\'', "''")) } +/// How a shell spells "run the commands in this file", for the shells whose +/// syntax herdr knows. +/// +/// `None` means herdr must not rewrite a command for this shell. `csh`, `nu`, +/// `elvish` and `xonsh` are all accepted pane shells that spell sourcing +/// differently, and typing `.` at them would break a command that would +/// otherwise have run. +#[cfg(unix)] +pub(crate) fn shell_source_keyword(shell_name: &str) -> Option<&'static str> { + match normalized_process_name(shell_name).as_str() { + "sh" | "bash" | "dash" | "zsh" | "ksh" | "mksh" => Some("."), + "fish" => Some("source"), + _ => None, + } +} + pub(crate) fn is_pane_shell_process_name(name: &str) -> bool { let normalized = normalized_process_name(name); matches!( diff --git a/tests/long_command_delivery.rs b/tests/long_command_delivery.rs new file mode 100644 index 0000000000..df64f2dbe1 --- /dev/null +++ b/tests/long_command_delivery.rs @@ -0,0 +1,377 @@ +//! `agent start` must deliver a composed command whatever its length. +//! +//! herdr runs an agent by typing its command at the pane's shell. A terminal in +//! canonical mode assembles one input line in a fixed buffer -- 1024 bytes on +//! Darwin, 4096 on Linux, both measured -- and hands the reader nothing until a +//! terminator arrives, so a longer line cannot be delivered: the kernel keeps a +//! prefix, discards the rest, and still reports the write as successful. +//! +//! `dash` is the deterministic case. It has no line editor, so it never leaves +//! canonical mode, and it is `/bin/sh` on Ubuntu. `bash` and `zsh` are only +//! intermittently exposed -- during pane startup before their line editor is +//! up, and for the duration of every foreground command -- which is why the +//! original report looked timing-dependent (refs #2862). + +pub mod support; + +use std::fs; +use std::io::{BufRead, BufReader, Write}; +use std::os::unix::fs::PermissionsExt; +use std::os::unix::net::UnixStream; +use std::path::{Path, PathBuf}; +use std::sync::{Mutex, MutexGuard, OnceLock}; +use std::thread; +use std::time::{Duration, Instant, SystemTime, UNIX_EPOCH}; + +use portable_pty::{native_pty_system, Child, CommandBuilder, MasterPty, PtySize}; +use serde_json::{json, Value}; +use support::{ + cleanup_test_base, register_runtime_dir, register_spawned_herdr_pid, + unregister_spawned_herdr_pid, wait_for_socket, +}; + +/// Longer than the canonical line buffer on either platform, so one payload +/// reproduces the failure everywhere. +const OVERSIZED_ARG: usize = 4200; +/// Comfortably typed directly on any platform. +const ORDINARY_ARG: usize = 20; + +fn unique_test_dir() -> PathBuf { + let nanos = SystemTime::now() + .duration_since(UNIX_EPOCH) + .map(|d| d.as_nanos()) + .unwrap_or(0); + PathBuf::from(format!( + "/tmp/herdr-long-command-test-{}-{nanos}", + std::process::id() + )) +} + +struct SpawnedHerdr { + _master: Option>, + child: Box, +} + +impl Drop for SpawnedHerdr { + fn drop(&mut self) { + let pid = self.child.process_id(); + let _ = self.child.kill(); + drop(self._master.take()); + if let Some(pid) = pid { + let deadline = Instant::now() + Duration::from_secs(2); + while Instant::now() < deadline { + let mut status = 0; + let result = + unsafe { libc::waitpid(pid as libc::pid_t, &mut status, libc::WNOHANG) }; + if result == pid as libc::pid_t || result == -1 { + break; + } + thread::sleep(Duration::from_millis(20)); + } + unregister_spawned_herdr_pid(Some(pid)); + } + } +} + +fn test_lock() -> MutexGuard<'static, ()> { + static LOCK: OnceLock> = OnceLock::new(); + LOCK.get_or_init(|| Mutex::new(())) + .lock() + .unwrap_or_else(|poisoned| poisoned.into_inner()) +} + +/// A stand-in for the agent binary that records how many bytes of its argument +/// actually arrived. A command cut by the line discipline never runs it at all. +fn install_fake_agent(bin_dir: &Path, marker: &Path) { + fs::create_dir_all(bin_dir).unwrap(); + let agent = bin_dir.join("claude"); + fs::write( + &agent, + format!( + "#!/bin/sh\nprintf %s \"${{#1}}\" > '{}'\nsleep 30\n", + marker.display() + ), + ) + .unwrap(); + fs::set_permissions(&agent, fs::Permissions::from_mode(0o755)).unwrap(); +} + +fn spawn_server( + config_home: &Path, + runtime_dir: &Path, + api_socket_path: &Path, + shell: &str, + bin_dir: &Path, +) -> SpawnedHerdr { + fs::create_dir_all(config_home.join("herdr")).unwrap(); + fs::create_dir_all(runtime_dir).unwrap(); + register_runtime_dir(runtime_dir); + fs::write( + config_home.join("herdr/config.toml"), + "onboarding = false\n", + ) + .unwrap(); + + let pair = native_pty_system() + .openpty(PtySize { + rows: 24, + cols: 80, + pixel_width: 0, + pixel_height: 0, + }) + .unwrap(); + + let mut cmd = CommandBuilder::new(env!("CARGO_BIN_EXE_herdr")); + cmd.arg("server"); + cmd.env("XDG_CONFIG_HOME", config_home); + cmd.env("XDG_STATE_HOME", config_home.join("state")); + cmd.env("XDG_RUNTIME_DIR", runtime_dir); + cmd.env("HERDR_SOCKET_PATH", api_socket_path); + cmd.env_remove("HERDR_CLIENT_SOCKET_PATH"); + cmd.env("SHELL", shell); + cmd.env("PATH", format!("{}:/usr/bin:/bin", bin_dir.display())); + cmd.env_remove("HERDR_ENV"); + + let child = pair.slave.spawn_command(cmd).unwrap(); + register_spawned_herdr_pid(child.process_id()); + drop(pair.slave); + + SpawnedHerdr { + _master: Some(pair.master), + child, + } +} + +fn send_json_request(socket_path: &Path, id: &str, method: &str, params: Value) -> Value { + let mut stream = UnixStream::connect(socket_path).expect("should connect to API socket"); + let request = json!({ "id": id, "method": method, "params": params }); + writeln!(stream, "{request}").unwrap(); + let mut reader = BufReader::new(stream); + let mut response = String::new(); + reader.read_line(&mut response).unwrap(); + serde_json::from_str(&response).expect("response should be valid JSON") +} + +fn pane_read(socket_path: &Path, pane_id: &str) -> String { + let response = send_json_request( + socket_path, + "pane_read", + "pane.read", + json!({ "pane_id": pane_id, "source": "recent", "strip_ansi": true }), + ); + response["result"]["read"]["text"] + .as_str() + .map(|text| text.trim_end().to_string()) + .unwrap_or_else(|| format!("")) +} + +fn wait_for_marker(marker: &Path, timeout: Duration) -> Option { + let deadline = Instant::now() + timeout; + while Instant::now() < deadline { + if let Ok(contents) = fs::read_to_string(marker) { + if !contents.is_empty() { + return Some(contents); + } + } + thread::sleep(Duration::from_millis(50)); + } + None +} + +struct Fixture { + _spawned: SpawnedHerdr, + base: PathBuf, + api_socket: PathBuf, + pane_id: String, + marker: PathBuf, +} + +impl Drop for Fixture { + fn drop(&mut self) { + cleanup_test_base(&self.base); + } +} + +fn start_pane(label: &str, shell: &str) -> Fixture { + let base = unique_test_dir(); + let config_home = base.join("config"); + let runtime_dir = base.join("runtime"); + let api_socket = base.join("api.sock"); + let bin_dir = base.join("bin"); + let marker = base.join("agent-argv-len"); + install_fake_agent(&bin_dir, &marker); + let spawned = spawn_server(&config_home, &runtime_dir, &api_socket, shell, &bin_dir); + wait_for_socket(&api_socket, Duration::from_secs(10)); + let created = send_json_request( + &api_socket, + "workspace_create", + "workspace.create", + json!({ "label": label }), + ); + let pane_id = created["result"]["root_pane"]["pane_id"] + .as_str() + .expect("root pane id") + .to_string(); + // Let the shell settle so the pane starts from its steady state rather than + // the canonical window every pane is born in. + thread::sleep(Duration::from_millis(1500)); + Fixture { + _spawned: spawned, + base, + api_socket, + pane_id, + marker, + } +} + +/// Start an agent and assert herdr accepted the request, which is the report +/// the issue says cannot be trusted. +fn agent_start(fixture: &Fixture, arg_len: usize) { + let response = send_json_request( + &fixture.api_socket, + "agent_start", + "agent.start", + json!({ + "name": "probe", + "kind": "claude", + "pane_id": fixture.pane_id, + "args": ["A".repeat(arg_len)], + "timeout_ms": 20000, + }), + ); + assert!( + response.get("error").is_none(), + "agent.start should be accepted: {response}" + ); +} + +fn dash_path() -> Option { + ["/bin/dash", "/usr/bin/dash"] + .into_iter() + .find(|candidate| Path::new(candidate).exists()) + .map(str::to_string) +} + +/// refs #2862: the reported failure, made deterministic. `dash` never leaves +/// canonical mode, so the composed command meets the line cap every time +/// rather than only inside a startup window. +#[test] +fn agent_start_delivers_long_args_to_a_shell_that_never_leaves_canonical_mode() { + let _guard = test_lock(); + let Some(dash) = dash_path() else { + eprintln!("skipping: dash is not installed on this host"); + return; + }; + let fixture = start_pane("long-canonical", &dash); + + agent_start(&fixture, OVERSIZED_ARG); + + assert_eq!( + wait_for_marker(&fixture.marker, Duration::from_secs(15)).as_deref(), + Some(&OVERSIZED_ARG.to_string()[..]), + "herdr reported the agent as started but its command never ran intact; pane showed:\n{}", + pane_read(&fixture.api_socket, &fixture.pane_id) + ); +} + +/// Control: an ordinary command reaches the same always-canonical shell, so a +/// failure above is about length and not about `dash` or the harness. +#[test] +fn agent_start_delivers_ordinary_args_to_a_shell_that_never_leaves_canonical_mode() { + let _guard = test_lock(); + let Some(dash) = dash_path() else { + eprintln!("skipping: dash is not installed on this host"); + return; + }; + let fixture = start_pane("ordinary-canonical", &dash); + + agent_start(&fixture, ORDINARY_ARG); + + assert_eq!( + wait_for_marker(&fixture.marker, Duration::from_secs(15)).as_deref(), + Some(&ORDINARY_ARG.to_string()[..]), + "an ordinary agent command must reach the pane; pane showed:\n{}", + pane_read(&fixture.api_socket, &fixture.pane_id) + ); +} + +/// Control: a shell whose line editor holds the terminal in raw mode has no +/// per-line cap and delivered long commands before this fix. It must keep +/// doing so. +#[test] +fn agent_start_delivers_long_args_at_a_raw_mode_prompt() { + let _guard = test_lock(); + let fixture = start_pane("long-raw", "/bin/sh"); + + agent_start(&fixture, OVERSIZED_ARG); + + assert_eq!( + wait_for_marker(&fixture.marker, Duration::from_secs(15)).as_deref(), + Some(&OVERSIZED_ARG.to_string()[..]), + "a raw-mode prompt must still receive long commands; pane showed:\n{}", + pane_read(&fixture.api_socket, &fixture.pane_id) + ); +} + +/// Every payload herdr has staged anywhere under a fixture's state directory. +fn staged_payloads(base: &Path) -> Vec { + let mut found = Vec::new(); + let mut stack = vec![base.to_path_buf()]; + while let Some(dir) = stack.pop() { + let Ok(entries) = fs::read_dir(&dir) else { + continue; + }; + for entry in entries.flatten() { + let path = entry.path(); + if !path.is_dir() { + continue; + } + if path + .file_name() + .is_some_and(|name| name == "staged-commands") + { + found.extend( + fs::read_dir(&path) + .into_iter() + .flatten() + .flatten() + .map(|payload| payload.path()), + ); + } else { + stack.push(path); + } + } + } + found +} + +/// refs #2862: staging writes the composed command, system prompt and all, to +/// disk. A request rejected after that point must take the file with it rather +/// than leave it readable in the state directory. +#[test] +fn a_rejected_agent_start_leaves_no_staged_payload() { + let _guard = test_lock(); + let fixture = start_pane("rejected", "/bin/sh"); + let response = send_json_request( + &fixture.api_socket, + "agent_start", + "agent.start", + json!({ + "name": "probe", + "kind": "claude", + "pane_id": fixture.pane_id, + "args": ["A".repeat(OVERSIZED_ARG)], + "timeout_ms": 1, + }), + ); + assert_eq!( + response["error"]["code"].as_str(), + Some("invalid_agent_timeout"), + "an out-of-range timeout must be rejected: {response}" + ); + assert!( + staged_payloads(&fixture.base).is_empty(), + "a rejected request must not leave a staged payload: {:?}", + staged_payloads(&fixture.base) + ); +}