diff --git a/crates/maestro-daemon/src/session.rs b/crates/maestro-daemon/src/session.rs index 1373293..b633fcf 100644 --- a/crates/maestro-daemon/src/session.rs +++ b/crates/maestro-daemon/src/session.rs @@ -211,17 +211,22 @@ impl SessionManager { .unwrap_or(false); let id = Uuid::new_v4(); - self.inner - .tmux - .new_session(id, &effective_cwd, &req.command) - .map_err(SessionError::Tmux)?; - + // Build the FIFO + reader task *before* asking tmux to spawn the + // command — otherwise the child's first bytes race the separate + // pipe-pane call and get lost (NAP-253). `new_session_piped` chains + // `pipe-pane` into the same tmux invocation as `new-session`, so the + // pipe is attached before the child finishes `execve`. let fifo_path = self.inner.fifo_dir.join(format!("{id}.fifo")); let bridge = PtyBridge::start_with_replay(fifo_path.clone(), self.inner.replay_bytes).await?; self.inner .tmux - .pipe_pane(id, &pipe_pane_command(&fifo_path)) + .new_session_piped( + id, + &effective_cwd, + &req.command, + &pipe_pane_command(&fifo_path), + ) .map_err(SessionError::Tmux)?; let session = Session { diff --git a/crates/maestro-daemon/src/tmux.rs b/crates/maestro-daemon/src/tmux.rs index 6eb99d9..9ede484 100644 --- a/crates/maestro-daemon/src/tmux.rs +++ b/crates/maestro-daemon/src/tmux.rs @@ -92,7 +92,28 @@ impl Tmux { let name = session_name(id); let out = self .cmd() - .args(new_session_args(&name, cwd, command)) + .args(new_session_args(&name, cwd, command, None)) + .output()?; + check(out) + } + + /// Like [`Self::new_session`], but chains `pipe-pane -t maestro- + /// ` into the same tmux invocation. Use this from + /// `SessionManager::create` so the pipe is in place before the freshly + /// `exec`-ed child writes its first byte; calling `new_session` + a + /// separate `pipe_pane` opens a window in which initial output is + /// silently dropped (NAP-253). + pub fn new_session_piped( + &self, + id: Uuid, + cwd: &Path, + command: &str, + sink_command: &str, + ) -> Result<(), TmuxError> { + let name = session_name(id); + let out = self + .cmd() + .args(new_session_args(&name, cwd, command, Some(sink_command))) .output()?; check(out) } @@ -212,11 +233,18 @@ pub fn session_name(id: Uuid) -> String { format!("{SESSION_PREFIX}{id}") } -/// Args passed to `tmux` for `new_session`, excluding the binary itself and -/// any socket/tmpdir flags injected by [`Tmux::cmd`]. Split out so the argv -/// can be asserted without invoking tmux. -fn new_session_args(name: &str, cwd: &Path, command: &str) -> Vec { - vec![ +/// Args passed to `tmux` for `new_session` / `new_session_piped`, excluding +/// the binary itself and any socket/tmpdir flags injected by [`Tmux::cmd`]. +/// When `pipe_sink` is `Some`, a chained `pipe-pane -t ` is +/// appended so the pipe attaches in the same tmux invocation that creates +/// the session. +fn new_session_args( + name: &str, + cwd: &Path, + command: &str, + pipe_sink: Option<&str>, +) -> Vec { + let mut args = vec![ OsString::from("set-option"), OsString::from("-g"), OsString::from("default-terminal"), @@ -233,7 +261,17 @@ fn new_session_args(name: &str, cwd: &Path, command: &str) -> Vec { OsString::from("-c"), cwd.as_os_str().to_os_string(), OsString::from(command), - ] + ]; + if let Some(sink) = pipe_sink { + args.extend([ + OsString::from(";"), + OsString::from("pipe-pane"), + OsString::from("-t"), + OsString::from(name), + OsString::from(sink), + ]); + } + args } fn check(out: Output) -> Result<(), TmuxError> { @@ -253,7 +291,7 @@ mod tests { #[test] fn new_session_args_pass_256color_env_and_default_terminal() { - let args = new_session_args("maestro-abc", Path::new("/tmp/proj"), "sleep 30"); + let args = new_session_args("maestro-abc", Path::new("/tmp/proj"), "sleep 30", None); let strs: Vec<&str> = args.iter().map(|s| s.to_str().unwrap()).collect(); assert_eq!( strs, @@ -277,4 +315,22 @@ mod tests { ] ); } + + #[test] + fn new_session_args_chain_pipe_pane_when_sink_present() { + let args = new_session_args( + "maestro-abc", + Path::new("/tmp/proj"), + "sleep 30", + Some("cat >> '/tmp/fifo'"), + ); + let strs: Vec<&str> = args.iter().map(|s| s.to_str().unwrap()).collect(); + // The pipe-pane chain must come after new-session in the same argv + // so tmux attaches the pipe before the child writes its first byte + // (NAP-253). + assert_eq!( + &strs[strs.len() - 5..], + &[";", "pipe-pane", "-t", "maestro-abc", "cat >> '/tmp/fifo'"] + ); + } } diff --git a/crates/maestro-daemon/tests/session_manager.rs b/crates/maestro-daemon/tests/session_manager.rs index 28f827b..778a32b 100644 --- a/crates/maestro-daemon/tests/session_manager.rs +++ b/crates/maestro-daemon/tests/session_manager.rs @@ -108,6 +108,55 @@ async fn recover_adopts_existing_tmux_sessions() { assert_eq!(snap[0].command, "sleep 30"); } +/// Regression for NAP-253: the very first bytes a freshly-spawned command +/// writes must reach the replay buffer. Before the fix, `pipe-pane` was a +/// separate tmux invocation that ran after `new-session` + an async FIFO +/// setup, so the child's first output was buffered by tmux but never tee'd +/// into the FIFO. +#[tokio::test] +async fn create_captures_initial_output_in_replay_buffer() { + let fx = DaemonFixture::new("laptop"); + let cwd = TempDir::new().unwrap(); + + let s = fx + .manager + .create(CreateSessionRequest { + daemon_id: "laptop".into(), + cwd: cwd.path().to_path_buf(), + // The shell prints the marker before doing anything else and + // then sleeps so the session stays alive. Before NAP-253 this + // marker was the canonical case that got swallowed. + command: "sh -c 'echo NAP_253_FIRST_LINE_MARKER; exec sleep 30'".into(), + env: HashMap::new(), + metadata: HashMap::new(), + worktree: None, + }) + .await + .expect("create"); + + let want = b"NAP_253_FIRST_LINE_MARKER"; + let deadline = std::time::Instant::now() + Duration::from_secs(5); + let mut found = false; + while std::time::Instant::now() < deadline { + let (replay, _rx) = fx + .manager + .subscribe_pty_with_replay(s.id) + .await + .expect("subscribe"); + if replay.windows(want.len()).any(|w| w == want) { + found = true; + break; + } + tokio::time::sleep(Duration::from_millis(50)).await; + } + assert!( + found, + "first-line marker never reached the replay buffer — pipe-pane raced past the child's initial output", + ); + + fx.manager.kill(s.id).await.unwrap(); +} + /// Regression for NAP-228: after `recover`, PTY output must reach new /// subscribers. Before the fix tmux's stale `pipe-pane` (with `-o`) silently /// kept feeding the old FIFO and the revived manager's subscribers saw