diff --git a/AGENTS.md b/AGENTS.md index 4edfb3ee..c56804c7 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -113,6 +113,7 @@ external contributor guardrail. ### Windows VM validation The Windows VM is for final/manual Windows validation, not normal agent work. +Connect to it with the `windows-wirt` SSH alias. Use the single reusable checkout at `C:\work\repo`. Do not create additional persistent Herdr clones or worktrees on the VM. The Windows account is already @@ -121,11 +122,15 @@ named `herdr`, so avoid paths like `C:\Users\herdr\herdr`. Before validating a fix on Windows, sync or apply the Linux worktree changes into `C:\work\repo`, then run the needed Windows build or test commands there. Reuse the shared Rust caches under `C:\Users\herdr\.cargo` and -`C:\Users\herdr\.rustup`. Do not use WSL on the VM. +`C:\Users\herdr\.rustup`. Do not use WSL on the VM. The VM may have a newer +Zig on `PATH`; Herdr currently requires Zig 0.15.2, so set +`$env:ZIG = "C:\Users\herdr\zig-0.15.2\zig.exe"` before running Cargo commands +that build the vendored libghostty-vt. After validation, leave `C:\work\repo` clean. Remove temporary files and delete `C:\work\repo\target` when disk space is tight, but keep the shared Cargo and -Rustup caches. +Rustup caches. Unless Can explicitly asks to keep the patched tree for more +manual testing, reset `C:\work\repo` back to a clean checkout before finishing. ## Agent Detection Updates diff --git a/docs/next/CHANGELOG.md b/docs/next/CHANGELOG.md index 9fbf5170..9f5489cd 100644 --- a/docs/next/CHANGELOG.md +++ b/docs/next/CHANGELOG.md @@ -15,6 +15,7 @@ - Bumped the client/server protocol version to 15 for socket API placement mutation event and response compatibility. ### Fixed +- `prefix+e` scrollback editor panes now open on Windows without trying to run `/bin/sh`; Windows uses `VISUAL`, then `EDITOR`, then `notepad.exe` as the fallback editor. (#914) - `herdr pane split --current` now resolves to the calling Herdr pane instead of the UI-focused pane when run inside a pane. (#902) - Native Windows clients running inside Alacritty now preserve mouse reports and `ctrl+j` input instead of leaking mouse escape sequences into panes. `shift+enter` remains dependent on whether the outer terminal reports it as a distinct modified Enter key. (#792) - OMP integration state now recovers after resumed sessions such as `omp -c` and reports Ask/tool approval waits as blocked instead of leaving the pane working or stuck on the previous OMP session. (#879) diff --git a/src/app/input/navigate.rs b/src/app/input/navigate.rs index 35359cde..186947b6 100644 --- a/src/app/input/navigate.rs +++ b/src/app/input/navigate.rs @@ -820,14 +820,27 @@ impl App { let path = write_scrollback_temp_file(&scrollback)?; - let quoted_path = shell_quote(&path.display().to_string()); - let command = format!( - r#"scrollback_file={quoted_path}; eval "${{EDITOR:-vi}} \"\$scrollback_file\""; status=$?; rm -f "$scrollback_file"; exit $status"# - ); - if let Err(err) = self.spawn_pane_command(&command, vec![path.clone()]) { - let _ = fs::remove_file(&path); - return Err(err); - } + let argv = match crate::platform::scrollback_editor_argv(&path) { + Ok(argv) => argv, + Err(err) => { + let _ = fs::remove_file(&path); + return Err(err); + } + }; + let (env, _) = self.custom_command_env(); + let new_pane = match self.spawn_overlay_argv_command(&argv, None, env, vec![path.clone()]) { + Ok((_, new_pane)) => new_pane, + Err(err) => { + let _ = fs::remove_file(&path); + return Err(err); + } + }; + let terminal_id = new_pane.terminal.id.clone(); + self.terminal_runtimes + .insert(terminal_id.clone(), new_pane.runtime); + self.state + .remove_alias_shadowed_by_new_pane(new_pane.pane_id); + self.state.terminals.insert(terminal_id, new_pane.terminal); if let Some(public_pane_id) = self.public_pane_id(ws_idx, pane_id) { self.state.toast = Some(crate::app::state::ToastNotification { @@ -1677,22 +1690,6 @@ fn unique_scrollback_path(attempt: u32) -> std::path::PathBuf { )) } -fn shell_quote(value: &str) -> String { - if !value.is_empty() - && value.chars().all(|ch| { - ch.is_ascii_alphanumeric() - || matches!( - ch, - '@' | '%' | '_' | '+' | '=' | ':' | ',' | '.' | '/' | '-' - ) - }) - { - return value.to_string(); - } - - format!("'{}'", value.replace('\'', "'\\''")) -} - #[cfg(test)] mod tests { #[cfg(unix)] @@ -2797,6 +2794,13 @@ last_pane = "prefix+tab" assert!(content.contains("alpha")); assert!(content.contains("beta")); assert_eq!(app.state.mode, Mode::Terminal); + assert!( + app.state.terminals.values().any(|terminal| terminal + .launch_argv + .as_ref() + .is_some_and(|argv| argv.first().is_some_and(|program| program == "/bin/sh"))), + "scrollback editor should launch through argv overlay path" + ); let _ = std::fs::remove_file(output_path); } diff --git a/src/platform/fallback.rs b/src/platform/fallback.rs index 25bc761c..e9134a8b 100644 --- a/src/platform/fallback.rs +++ b/src/platform/fallback.rs @@ -6,6 +6,14 @@ use super::{ClipboardImage, ForegroundJob, Signal}; /// Unsupported platform stub. pub fn raise_server_nofile_limit() {} +/// Unsupported platform stub. +pub(crate) fn scrollback_editor_argv(_path: &std::path::Path) -> std::io::Result> { + Err(std::io::Error::new( + std::io::ErrorKind::Unsupported, + "opening scrollback in an editor is not supported on this platform", + )) +} + /// Unsupported platform stub. pub fn detach_server_daemon_command(_command: &mut Command) {} diff --git a/src/platform/linux.rs b/src/platform/linux.rs index 3cf643e5..539c0d93 100644 --- a/src/platform/linux.rs +++ b/src/platform/linux.rs @@ -12,6 +12,30 @@ use super::{ pub fn raise_server_nofile_limit() {} +pub(crate) fn scrollback_editor_argv(path: &std::path::Path) -> std::io::Result> { + let quoted_path = shell_quote(&path.display().to_string()); + let command = format!( + r#"scrollback_file={quoted_path}; eval "${{EDITOR:-vi}} \"\$scrollback_file\""; status=$?; rm -f "$scrollback_file"; exit $status"# + ); + Ok(vec!["/bin/sh".to_string(), "-c".to_string(), command]) +} + +fn shell_quote(value: &str) -> String { + if !value.is_empty() + && value.chars().all(|ch| { + ch.is_ascii_alphanumeric() + || matches!( + ch, + '@' | '%' | '_' | '+' | '=' | ':' | ',' | '.' | '/' | '-' + ) + }) + { + return value.to_string(); + } + + format!("'{}'", value.replace('\'', "'\\''")) +} + /// Collect the foreground terminal job for a given child PID. pub fn foreground_job(child_pid: u32) -> Option { let tpgid = foreground_process_group_id(child_pid)?; @@ -794,4 +818,15 @@ mod tests { let _ = std::fs::remove_file(&path); assert_eq!(args, "--\n-danger\nbody\n"); } + + #[test] + fn scrollback_editor_argv_preserves_unix_editor_shell_semantics() { + let path = std::path::Path::new("/tmp/herdr scrollback.txt"); + let argv = scrollback_editor_argv(path).unwrap(); + + assert_eq!(argv[0], "/bin/sh"); + assert_eq!(argv[1], "-c"); + assert!(argv[2].contains("EDITOR:-vi")); + assert!(argv[2].contains("/tmp/herdr scrollback.txt")); + } } diff --git a/src/platform/macos.rs b/src/platform/macos.rs index df957652..3770a3be 100644 --- a/src/platform/macos.rs +++ b/src/platform/macos.rs @@ -16,6 +16,30 @@ const PROC_PGRP_ONLY: u32 = 2; const SERVER_NOFILE_LIMIT_TARGET: libc::rlim_t = 8192; const CF_STRING_ENCODING_UTF8: u32 = 0x0800_0100; +pub(crate) fn scrollback_editor_argv(path: &Path) -> std::io::Result> { + let quoted_path = shell_quote(&path.display().to_string()); + let command = format!( + r#"scrollback_file={quoted_path}; eval "${{EDITOR:-vi}} \"\$scrollback_file\""; status=$?; rm -f "$scrollback_file"; exit $status"# + ); + Ok(vec!["/bin/sh".to_string(), "-c".to_string(), command]) +} + +fn shell_quote(value: &str) -> String { + if !value.is_empty() + && value.chars().all(|ch| { + ch.is_ascii_alphanumeric() + || matches!( + ch, + '@' | '%' | '_' | '+' | '=' | ':' | ',' | '.' | '/' | '-' + ) + }) + { + return value.to_string(); + } + + format!("'{}'", value.replace('\'', "'\\''")) +} + #[repr(C)] struct TisInputSource { _private: [u8; 0], @@ -1117,4 +1141,15 @@ printf '%s\n' "$@" > "$HERDR_NOTIFY_ARGS" "-e\non run argv\n-e\ndisplay notification (item 2 of argv) with title (item 1 of argv)\n-e\nend run\ntitle\nbody\n" ); } + + #[test] + fn scrollback_editor_argv_preserves_unix_editor_shell_semantics() { + let path = std::path::Path::new("/tmp/herdr scrollback.txt"); + let argv = scrollback_editor_argv(path).unwrap(); + + assert_eq!(argv[0], "/bin/sh"); + assert_eq!(argv[1], "-c"); + assert!(argv[2].contains("EDITOR:-vi")); + assert!(argv[2].contains("/tmp/herdr scrollback.txt")); + } } diff --git a/src/platform/windows.rs b/src/platform/windows.rs index e7a77e35..50e0e905 100644 --- a/src/platform/windows.rs +++ b/src/platform/windows.rs @@ -46,6 +46,41 @@ struct WindowsProcessEntry { pub fn raise_server_nofile_limit() {} +pub(crate) fn scrollback_editor_argv(path: &std::path::Path) -> std::io::Result> { + let editor = std::env::var("VISUAL") + .ok() + .filter(|value| !value.trim().is_empty()) + .or_else(|| { + std::env::var("EDITOR") + .ok() + .filter(|value| !value.trim().is_empty()) + }); + scrollback_editor_argv_with_env(path, editor.as_deref()) +} + +fn scrollback_editor_argv_with_env( + path: &std::path::Path, + editor: Option<&str>, +) -> std::io::Result> { + let mut argv = match editor.filter(|value| !value.trim().is_empty()) { + Some(editor) => command_line_to_argv(editor).ok_or_else(|| { + std::io::Error::new( + std::io::ErrorKind::InvalidInput, + format!("failed to parse editor command {editor:?}"), + ) + })?, + None => vec!["notepad.exe".to_string()], + }; + if argv.is_empty() { + return Err(std::io::Error::new( + std::io::ErrorKind::InvalidInput, + "editor command must not be empty", + )); + } + argv.push(path.display().to_string()); + Ok(argv) +} + pub fn detach_server_daemon_command(_command: &mut std::process::Command) {} pub fn current_process_is_detached_server_daemon() -> bool { @@ -698,6 +733,31 @@ mod tests { assert_eq!(pids, vec![10, 20, 30]); } + #[test] + fn scrollback_editor_argv_uses_editor_env_and_appends_path() { + let path = std::path::Path::new(r"C:\Users\User\AppData\Local\Temp\herdr scrollback.txt"); + let argv = super::scrollback_editor_argv_with_env( + path, + Some(r#""C:\Program Files\Microsoft VS Code\Code.exe" --wait"#), + ) + .unwrap(); + + assert_eq!(argv[0], r"C:\Program Files\Microsoft VS Code\Code.exe"); + assert_eq!(argv[1], "--wait"); + assert_eq!(argv[2], path.display().to_string()); + } + + #[test] + fn scrollback_editor_argv_falls_back_to_notepad() { + let path = std::path::Path::new(r"C:\Temp\herdr-scrollback.txt"); + let argv = super::scrollback_editor_argv_with_env(path, None).unwrap(); + + assert_eq!( + argv, + vec!["notepad.exe".to_string(), path.display().to_string()] + ); + } + fn test_entry( pid: u32, parent_pid: u32,