From 78a8d15031a2cbc0467ce285264ff21a8c5953bf Mon Sep 17 00:00:00 2001 From: "adam.paterson" Date: Wed, 23 Sep 2026 22:08:50 +0100 Subject: [PATCH] fix(herdr): preserve named sessions for Herdr callbacks --- crates/plannotator-tui/src/cli.rs | 2 +- crates/plannotator-tui/src/delivery.rs | 43 +++++++++++++++++-- crates/plannotator-tui/src/herdr/context.rs | 37 +++++++++++++++- crates/plannotator-tui/src/herdr/launch.rs | 10 ++--- crates/plannotator-tui/src/last/locate.rs | 3 +- .../tests/support/fake-herdr.rs | 8 +++- .../tests/windows_subprocess.rs | 10 ++++- 7 files changed, 98 insertions(+), 15 deletions(-) diff --git a/crates/plannotator-tui/src/cli.rs b/crates/plannotator-tui/src/cli.rs index 80a59fc..0a7a741 100644 --- a/crates/plannotator-tui/src/cli.rs +++ b/crates/plannotator-tui/src/cli.rs @@ -60,7 +60,7 @@ pub(crate) fn delivery(interactive: bool) -> Box { match env.delivery_target() { Some(target) if env.in_herdr => { let agent = target.agent.or_else(|| env.agent_in_pane(&target.pane)); - Box::new(HerdrAgent::new(env.bin, target.pane, agent)) + Box::new(HerdrAgent::with_session(env.bin, target.pane, agent, env.session_name)) } _ => Box::new(Clipboard), } diff --git a/crates/plannotator-tui/src/delivery.rs b/crates/plannotator-tui/src/delivery.rs index 3ea5694..59ae05c 100644 --- a/crates/plannotator-tui/src/delivery.rs +++ b/crates/plannotator-tui/src/delivery.rs @@ -7,7 +7,9 @@ use std::io::Write as _; use std::path::PathBuf; -use std::process::{Command, Stdio}; +use std::process::Stdio; + +use crate::herdr::context::herdr_command; /// Why a send did not land. The app reacts differently to each. #[derive(Debug)] @@ -98,11 +100,22 @@ pub(crate) struct HerdrAgent { bin: PathBuf, pane: String, agent: Option, + session_name: Option, } impl HerdrAgent { + #[cfg(test)] pub(crate) fn new(bin: PathBuf, pane: String, agent: Option) -> Self { - Self { bin, pane, agent } + Self { bin, pane, agent, session_name: None } + } + + pub(crate) fn with_session( + bin: PathBuf, + pane: String, + agent: Option, + session_name: Option, + ) -> Self { + Self { bin, pane, agent, session_name } } } @@ -123,7 +136,7 @@ impl Delivery for HerdrAgent { } fn deliver(&self, feedback: &str) -> Result<(), DeliveryError> { - let output = Command::new(&self.bin) + let output = herdr_command(&self.bin, self.session_name.as_deref()) .args(["agent", "prompt", &self.pane, feedback]) .stdin(Stdio::null()) .output() @@ -246,6 +259,30 @@ mod tests { assert_eq!(HerdrAgent::new(bin, "w1:p1".into(), None).describe(), "w1:p1"); } + #[cfg(unix)] + #[test] + fn named_herdr_session_is_passed_to_agent_prompt() { + use std::os::unix::fs::PermissionsExt; + + let root = std::env::temp_dir().join(format!("plannotator-delivery-session-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&root); + std::fs::create_dir_all(&root).expect("temp root"); + let fake = root.join("fake-herdr"); + let log = root.join("argv"); + std::fs::write(&fake, format!("#!/bin/sh\nprintf '%s\\n' \"$@\" > {}\n", log.display())) + .expect("fake herdr"); + std::fs::set_permissions(&fake, std::fs::Permissions::from_mode(0o700)).expect("chmod"); + + HerdrAgent::with_session(fake, "w1:p1".into(), Some("codex".into()), Some("personal".into())) + .deliver("feedback") + .expect("delivered"); + assert_eq!( + std::fs::read_to_string(&log).expect("log"), + "--session\npersonal\nagent\nprompt\nw1:p1\nfeedback\n" + ); + std::fs::remove_dir_all(root).expect("cleanup"); + } + #[cfg(windows)] #[test] fn windows_create_process_keeps_feedback_in_one_argument() { diff --git a/crates/plannotator-tui/src/herdr/context.rs b/crates/plannotator-tui/src/herdr/context.rs index 6abac4e..023ab48 100644 --- a/crates/plannotator-tui/src/herdr/context.rs +++ b/crates/plannotator-tui/src/herdr/context.rs @@ -1,6 +1,7 @@ //! The environment Herdr hands a plugin process, and the delivery target derived from it. -use std::path::PathBuf; +use std::path::{Path, PathBuf}; +use std::process::Command; use serde::Deserialize; @@ -33,6 +34,8 @@ pub(crate) struct HerdrEnv { pub(crate) in_herdr: bool, /// `HERDR_BIN_PATH`, else `herdr` on `PATH`. pub(crate) bin: PathBuf, + /// `HERDR_SESSION`: Herdr's named session. Explicit `--session` beats inherited socket overrides. + pub(crate) session_name: Option, /// `HERDR_PANE_ID`: the pane this process runs in. Inside the app that is our own /// pane; only the launcher may treat it as "the caller". pub(crate) pane_id: Option, @@ -71,6 +74,7 @@ impl HerdrEnv { Self { in_herdr: env("HERDR_ENV").as_deref() == Some("1"), bin: non_empty("HERDR_BIN_PATH").map_or_else(|| PathBuf::from("herdr"), PathBuf::from), + session_name: non_empty("HERDR_SESSION"), pane_id: non_empty("HERDR_PANE_ID"), context, file: non_empty("PLANNOTATOR_TUI_FILE").map(PathBuf::from), @@ -110,7 +114,7 @@ impl HerdrEnv { /// Ask Herdr which agent runs in `pane` (`herdr pane get`), for the label when the /// launcher only knew the pane id. One short process at startup; `None` on any failure. pub(crate) fn agent_in_pane(&self, pane: &str) -> Option { - let output = std::process::Command::new(&self.bin) + let output = herdr_command(&self.bin, self.session_name.as_deref()) .args(["pane", "get", pane]) .stdin(std::process::Stdio::null()) .output() @@ -121,6 +125,14 @@ impl HerdrEnv { } } +pub(crate) fn herdr_command(bin: &Path, session_name: Option<&str>) -> Command { + let mut command = Command::new(bin); + if let Some(session) = session_name.filter(|session| !session.is_empty()) { + command.args(["--session", session]); + } + command +} + #[cfg(test)] #[allow(clippy::expect_used, reason = "tests assert by panicking")] mod tests { @@ -146,9 +158,30 @@ mod tests { let env = env(&[]); assert!(!env.in_herdr); assert_eq!(env.bin, PathBuf::from("herdr")); + assert_eq!(env.session_name, None); assert_eq!(env.delivery_target(), None); } + #[test] + fn named_herdr_session_is_preserved_for_child_cli_calls() { + let named = env(&[("HERDR_SESSION", "personal")]); + assert_eq!(named.session_name.as_deref(), Some("personal")); + assert_eq!(env(&[("HERDR_SESSION", "")]).session_name, None); + } + + #[test] + fn herdr_command_prepends_explicit_session_when_present() { + let mut command = herdr_command(Path::new("herdr"), Some("personal")); + command.args(["pane", "list"]); + let args: Vec<_> = command.get_args().map(|arg| arg.to_string_lossy().into_owned()).collect(); + assert_eq!(args, ["--session", "personal", "pane", "list"]); + + let mut command = herdr_command(Path::new("herdr"), None); + command.args(["pane", "list"]); + let args: Vec<_> = command.get_args().map(|arg| arg.to_string_lossy().into_owned()).collect(); + assert_eq!(args, ["pane", "list"]); + } + #[test] fn explicit_deliver_to_wins_and_carries_the_agent_label() { let env = env(&[ diff --git a/crates/plannotator-tui/src/herdr/launch.rs b/crates/plannotator-tui/src/herdr/launch.rs index 8e21471..c6c97dc 100644 --- a/crates/plannotator-tui/src/herdr/launch.rs +++ b/crates/plannotator-tui/src/herdr/launch.rs @@ -3,12 +3,11 @@ //! and agents (the skill). `plan` and `argv` are pure; only `run` touches a process. use std::path::{Path, PathBuf}; -use std::process::Command; use anyhow::{Context, Result}; use plannotator_tui_hosts::Host; -use super::context::{HerdrEnv, Target}; +use super::context::{HerdrEnv, Target, herdr_command}; use crate::config::{Config, Placement, SplitDirection}; /// Command-line inputs to the launcher. @@ -309,7 +308,7 @@ pub(crate) fn argv(launch: &Launch) -> Vec { /// `herdr pane process-info --pane `, raw JSON. pub(crate) fn process_info(env: &HerdrEnv, pane: &str) -> Result { - let output = Command::new(&env.bin) + let output = herdr_command(&env.bin, env.session_name.as_deref()) .args(["pane", "process-info", "--pane", pane]) .output() .with_context(|| format!("running {} pane process-info", env.bin.display()))?; @@ -321,7 +320,8 @@ pub(crate) fn process_info(env: &HerdrEnv, pane: &str) -> Result { /// `herdr agent get `, raw JSON; `None` when the pane has no agent Herdr can describe. pub(crate) fn agent_get(env: &HerdrEnv, pane: &str) -> Option { - let output = Command::new(&env.bin).args(["agent", "get", pane]).output().ok()?; + let output = + herdr_command(&env.bin, env.session_name.as_deref()).args(["agent", "get", pane]).output().ok()?; output.status.success().then(|| String::from_utf8_lossy(&output.stdout).into_owned()) } @@ -330,7 +330,7 @@ pub(crate) fn run(env: &HerdrEnv, launch: &Launch) -> Result<()> { if !env.in_herdr { anyhow::bail!("not inside Herdr (HERDR_ENV is not set)"); } - let status = Command::new(&env.bin) + let status = herdr_command(&env.bin, env.session_name.as_deref()) .args(argv(launch)) .status() .with_context(|| format!("running {}", env.bin.display()))?; diff --git a/crates/plannotator-tui/src/last/locate.rs b/crates/plannotator-tui/src/last/locate.rs index e47d281..8dc92a0 100644 --- a/crates/plannotator-tui/src/last/locate.rs +++ b/crates/plannotator-tui/src/last/locate.rs @@ -1,7 +1,6 @@ //! Select the exact-session or fallback discovery path and read its assistant messages. use std::path::{Path, PathBuf}; -use std::process::Command; use anyhow::{Context, Result, bail}; use plannotator_tui_hosts::{Host, HostError, Message, Role, detect_host, sniff}; @@ -125,7 +124,7 @@ pub(crate) fn screen_fallback(env: &crate::herdr::context::HerdrEnv) -> Option>().as_slice() { + let arg_refs: Vec<&str> = args.iter().map(String::as_str).collect(); + let command_args = match arg_refs.as_slice() { + ["--session", _, rest @ ..] => rest, + rest => rest, + }; + + match command_args { ["agent", "get", _] => print!( "{}", fixture( diff --git a/crates/plannotator-tui/tests/windows_subprocess.rs b/crates/plannotator-tui/tests/windows_subprocess.rs index 294882b..8f217ec 100644 --- a/crates/plannotator-tui/tests/windows_subprocess.rs +++ b/crates/plannotator-tui/tests/windows_subprocess.rs @@ -84,6 +84,8 @@ fn clicked_file_and_exact_session_launches_preserve_argv_without_process_info() let open = bin() .env("HERDR_ENV", "1") .env("HERDR_BIN_PATH", &fake) + .env("HERDR_SESSION", "personal") + .env("HERDR_SOCKET_PATH", r"C:\wrong\default.sock") .env("HERDR_PLUGIN_ID", "annotate") .env("HERDR_PLUGIN_ROOT", &plugin_root) .env("HERDR_PLUGIN_CONTEXT_JSON", context(&clicked_root, Some(&clicked_url))) @@ -95,6 +97,8 @@ fn clicked_file_and_exact_session_launches_preserve_argv_without_process_info() let last = bin() .env("HERDR_ENV", "1") .env("HERDR_BIN_PATH", &fake) + .env("HERDR_SESSION", "personal") + .env("HERDR_SOCKET_PATH", r"C:\wrong\default.sock") .env("HERDR_PLUGIN_ID", "annotate") .env("HERDR_PLUGIN_ROOT", &plugin_root) .env("HERDR_PLUGIN_CONTEXT_JSON", context(&clicked_root, None)) @@ -107,6 +111,8 @@ fn clicked_file_and_exact_session_launches_preserve_argv_without_process_info() assert_eq!( calls[0]["argv"], json!([ + "--session", + "personal", "plugin", "pane", "open", @@ -127,10 +133,12 @@ fn clicked_file_and_exact_session_launches_preserve_argv_without_process_info() "PLANNOTATOR_TUI_DELIVER_AGENT=codex", ]) ); - assert_eq!(calls[1]["argv"], json!(["agent", "get", "w1:p1"])); + assert_eq!(calls[1]["argv"], json!(["--session", "personal", "agent", "get", "w1:p1"])); assert_eq!( calls[2]["argv"], json!([ + "--session", + "personal", "plugin", "pane", "open",