diff --git a/README.md b/README.md index 82c20394..900f1943 100644 --- a/README.md +++ b/README.md @@ -188,8 +188,8 @@ prints a session's effective timeouts. | Command | Description | | ------------------------------------------------------------ | ------------------------------------------- | -| `open [--shell S] [--cols N --rows N] [--cwd D] [--env K=V] [--config F] [--profile P] [--timeout- MS]` | Spawn a shell session. | -| `run [--config F] [--profile P] [args...]` | Spawn a session running a program directly. | +| `open [--shell S] [--cols N --rows N] [--cwd D] [--env K=V] [--config F] [--profile P] [--timeout- MS] [--restart]` | Spawn or reuse a shell session. | +| `run [--config F] [--profile P] [--restart] [args...]` | Spawn or reuse a session running a program. | | `sessions` | List active sessions. | | `close [--all]` | Close the current session (or all). | | `daemon start` / `daemon status` / `daemon stop --session N \| --all` | Start, inspect, or stop a session's daemon. | @@ -197,11 +197,18 @@ prints a session's effective timeouts. Each session has its own daemon, so `daemon stop` needs `--session ` or `--all`. `close` stops it too. +When a client finds a daemon from another `tui-test` version, it shuts that +daemon down and starts the current version before sending the command. The +restart is serialized per session so concurrent clients cannot race. + `open` waits for a prompt before returning, `run` does not. Override with `--wait-ready` / `--no-wait-ready`. An explicit `--wait-ready` fails (exit 1) if no prompt appears; `open`'s implicit wait reports `ready` in its payload either way. +Calling `open` or `run` for a session that already has a live child reuses that +child. Pass `--restart` (or its `--force` alias) to replace it explicitly. + ### Inspection | Command | Description | diff --git a/SKILL.md b/SKILL.md index 7a53029e..0d70273a 100644 --- a/SKILL.md +++ b/SKILL.md @@ -35,6 +35,9 @@ Three commands let an agent look up the rest of the surface instead of guessing: (`assertion` / `usage` / `no_session` / `internal`). - **Verbose.** `--verbose` / `-v` starts the daemon with a full PTY traffic log (see [Debugging](#debugging)). Only takes effect when the daemon starts. +- **Daemon upgrades.** A client automatically replaces a daemon from another + `tui-test` version. Per-session locking prevents concurrent clients from + racing the restart. - **Defaults.** New sessions are `80x30`. Timeouts come in five classes: `text` and `idle` default to 5s; `command`, `exit`, and `ready` to 30s. Set a session default with `open --timeout- `, or override one call with @@ -60,14 +63,17 @@ without parsing text: | Command | Description | | ------------------------------------------------------------------------ | ---------------------------------------------------------------------- | -| `open [--shell S] [--cols N] [--rows N] [--cwd D] [--env K=V]... [--config F] [--profile P]` | Spawn a shell session (auto-starts the daemon). `--env` is repeatable. | -| `run [--cols N] [--rows N] [--cwd D] [--env K=V]... [--config F] [--profile P] [args...]` | Spawn a session running a program directly (no shell). | +| `open [--shell S] [--cols N] [--rows N] [--cwd D] [--env K=V]... [--config F] [--profile P] [--restart]` | Spawn or reuse a shell session. `--env` is repeatable. | +| `run [--cols N] [--rows N] [--cwd D] [--env K=V]... [--config F] [--profile P] [--restart] [args...]` | Spawn or reuse a session running a program directly. | | `sessions` | List active sessions. | | `close [--all]` | Close the current session (or every session with `--all`). | | `daemon start` | Start this session's daemon. Most commands start one on demand. | | `daemon status` | Inspect a session's daemon (pid, log path). Exit 3 if none is running. | | `daemon stop --session N \| --all` | Stop one session's daemon, or every daemon. Needs a target. | +`open` and `run` reuse an existing live child for the selected session. Pass +`--restart` (or `--force`) to replace it. + ### Inspection | Command | Description | diff --git a/bindings/js/README.md b/bindings/js/README.md index a589b71b..8bab64c8 100644 --- a/bindings/js/README.md +++ b/bindings/js/README.md @@ -54,7 +54,8 @@ All derive from `TuiTestError` and carry `kind` and `exitCode`. `waitX` and `exp Module-level helpers: `sessions()`, `closeAll()`, `getRecording()`, `uniqueSession()`. `open` and `run` accept -`{ cols, rows, cwd, env, waitReady, retries, profile, timeouts }`. The +`{ cols, rows, cwd, env, waitReady, restart, retries, profile, timeouts }`. +They reuse a live named session unless `restart: true` is passed. The constructor also accepts `profile` as the default for later opens and runs. Profiles are partial; omitted fields use the built-in defaults: diff --git a/bindings/js/native/index.d.ts b/bindings/js/native/index.d.ts index 3501a82f..cbd35746 100644 --- a/bindings/js/native/index.d.ts +++ b/bindings/js/native/index.d.ts @@ -107,6 +107,7 @@ export interface OpenOptions { cwd?: string env?: Array<[string, string]> waitReady?: boolean + restart?: boolean profileScrollback?: number profileColors?: Array<[string, string]> timeouts?: Timeouts @@ -145,6 +146,7 @@ export interface RunOptions { cwd?: string env?: Array<[string, string]> waitReady?: boolean + restart?: boolean profileScrollback?: number profileColors?: Array<[string, string]> timeouts?: Timeouts diff --git a/bindings/js/native/lib.rs b/bindings/js/native/lib.rs index 3693b045..62ddf06e 100644 --- a/bindings/js/native/lib.rs +++ b/bindings/js/native/lib.rs @@ -66,6 +66,7 @@ pub struct OpenOptions { pub cwd: Option, pub env: Option>, pub wait_ready: Option, + pub restart: Option, pub profile_scrollback: Option, pub profile_colors: Option>, pub timeouts: Option, @@ -80,6 +81,7 @@ pub struct RunOptions { pub cwd: Option, pub env: Option>, pub wait_ready: Option, + pub restart: Option, pub profile_scrollback: Option, pub profile_colors: Option>, pub timeouts: Option, @@ -496,6 +498,7 @@ fn open_options(value: Option) -> std::result::Result std::result::Result | [string, string][]; waitReady?: boolean; + restart?: boolean; retries?: number; profile?: Profile; timeouts?: Timeouts; diff --git a/bindings/js/test/options.test.mjs b/bindings/js/test/options.test.mjs index e86fc68c..b4948b86 100644 --- a/bindings/js/test/options.test.mjs +++ b/bindings/js/test/options.test.mjs @@ -157,6 +157,7 @@ test("constructor and per-run profile objects recolor the terminal", async () => await su.expectText("constructor-profile", { fg: "#010203" }); await su.run(process.execPath, argsFor("call-profile"), { + restart: true, profile: { colors: { red: "#040506" } }, }); await su.waitText("call-profile", { timeout: 5000 }); diff --git a/bindings/python/README.md b/bindings/python/README.md index dab478dc..97d9df76 100644 --- a/bindings/python/README.md +++ b/bindings/python/README.md @@ -58,9 +58,10 @@ All derive from `TuiTestError`. `wait_*` and `expect_*` raise `ExpectationError` Module-level helpers: `sessions()`, `close_all()`, `get_recording()`, `unique_session()`. -`open()` and `run()` accept `wait_ready=`, `retries=`, `profile=`, and -`timeouts=`. The constructor also accepts `profile=` as the default for later -opens and runs. Profiles are partial; omitted fields use the built-in defaults: +`open()` and `run()` accept `wait_ready=`, `restart=`, `retries=`, `profile=`, +and `timeouts=`. They reuse a live named session unless `restart=True` is +passed. The constructor also accepts `profile=` as the default for later opens +and runs. Profiles are partial; omitted fields use the built-in defaults: ```python from tui_test import Colors, Profile, TuiTest diff --git a/bindings/python/native/src/lib.rs b/bindings/python/native/src/lib.rs index bce23500..02a3b828 100644 --- a/bindings/python/native/src/lib.rs +++ b/bindings/python/native/src/lib.rs @@ -58,6 +58,7 @@ impl NativeSession { cwd, env, wait_ready, + restart, profile_scrollback, profile_colors, text_timeout, @@ -76,6 +77,7 @@ impl NativeSession { cwd: Option, env: Vec<(String, String)>, wait_ready: Option, + restart: bool, profile_scrollback: Option>, profile_colors: Vec<(String, String)>, text_timeout: Option>, @@ -106,6 +108,7 @@ impl NativeSession { cwd, env, wait_ready, + restart, timeouts: Timeouts { text: optional_u64(text_timeout.as_ref(), "text_timeout")?, idle: optional_u64(idle_timeout.as_ref(), "idle_timeout")?, @@ -128,6 +131,7 @@ impl NativeSession { cwd, env, wait_ready, + restart, profile_scrollback, profile_colors, text_timeout, @@ -147,6 +151,7 @@ impl NativeSession { cwd: Option, env: Vec<(String, String)>, wait_ready: Option, + restart: bool, profile_scrollback: Option>, profile_colors: Vec<(String, String)>, text_timeout: Option>, @@ -178,6 +183,7 @@ impl NativeSession { cwd, env, wait_ready, + restart, timeouts: Timeouts { text: optional_u64(text_timeout.as_ref(), "text_timeout")?, idle: optional_u64(idle_timeout.as_ref(), "idle_timeout")?, diff --git a/bindings/python/src/tui_test/_native.pyi b/bindings/python/src/tui_test/_native.pyi index 1d3bc57d..43157fb2 100644 --- a/bindings/python/src/tui_test/_native.pyi +++ b/bindings/python/src/tui_test/_native.pyi @@ -38,8 +38,8 @@ class NativeSession: @property def name(self) -> builtins.str: ... def __new__(cls, name: str) -> NativeSession: ... - def open(self, shell: typing.Optional[str], cols: int, rows: int, cwd: typing.Optional[str], env: typing.List[typing.Tuple[str, str]], wait_ready: typing.Optional[bool], profile_scrollback: typing.Optional[int], profile_colors: typing.List[typing.Tuple[str, str]], text_timeout: typing.Optional[int], idle_timeout: typing.Optional[int], command_timeout: typing.Optional[int], exit_timeout: typing.Optional[int], ready_timeout: typing.Optional[int]) -> typing.Awaitable[typing.Dict[str, typing.Any]]: ... - def run(self, program: str, args: typing.List[str], cols: int, rows: int, cwd: typing.Optional[str], env: typing.List[typing.Tuple[str, str]], wait_ready: typing.Optional[bool], profile_scrollback: typing.Optional[int], profile_colors: typing.List[typing.Tuple[str, str]], text_timeout: typing.Optional[int], idle_timeout: typing.Optional[int], command_timeout: typing.Optional[int], exit_timeout: typing.Optional[int], ready_timeout: typing.Optional[int]) -> typing.Awaitable[typing.Dict[str, typing.Any]]: ... + def open(self, shell: typing.Optional[str], cols: int, rows: int, cwd: typing.Optional[str], env: typing.List[typing.Tuple[str, str]], wait_ready: typing.Optional[bool], restart: bool, profile_scrollback: typing.Optional[int], profile_colors: typing.List[typing.Tuple[str, str]], text_timeout: typing.Optional[int], idle_timeout: typing.Optional[int], command_timeout: typing.Optional[int], exit_timeout: typing.Optional[int], ready_timeout: typing.Optional[int]) -> typing.Awaitable[typing.Dict[str, typing.Any]]: ... + def run(self, program: str, args: typing.List[str], cols: int, rows: int, cwd: typing.Optional[str], env: typing.List[typing.Tuple[str, str]], wait_ready: typing.Optional[bool], restart: bool, profile_scrollback: typing.Optional[int], profile_colors: typing.List[typing.Tuple[str, str]], text_timeout: typing.Optional[int], idle_timeout: typing.Optional[int], command_timeout: typing.Optional[int], exit_timeout: typing.Optional[int], ready_timeout: typing.Optional[int]) -> typing.Awaitable[typing.Dict[str, typing.Any]]: ... def close(self) -> typing.Awaitable[None]: ... def state(self) -> typing.Awaitable[typing.Dict[str, typing.Any]]: ... def text(self, full: bool) -> typing.Awaitable[str]: ... diff --git a/bindings/python/src/tui_test/client.py b/bindings/python/src/tui_test/client.py index 5c55b953..dedaf963 100644 --- a/bindings/python/src/tui_test/client.py +++ b/bindings/python/src/tui_test/client.py @@ -231,6 +231,7 @@ async def open( cwd: Optional[str] = None, env: EnvLike = None, wait_ready: Optional[bool] = None, + restart: bool = False, profile: Optional[Profile] = None, timeouts: Optional[Timeouts] = None, retries: int = 0, @@ -248,6 +249,7 @@ async def open( cwd, env_values, wait_ready, + restart, *profile_values, *timeout_values, ), @@ -263,6 +265,7 @@ async def run( cwd: Optional[str] = None, env: EnvLike = None, wait_ready: Optional[bool] = None, + restart: bool = False, profile: Optional[Profile] = None, timeouts: Optional[Timeouts] = None, retries: int = 0, @@ -281,6 +284,7 @@ async def run( cwd, env_values, wait_ready, + restart, *profile_values, *timeout_values, ), diff --git a/bindings/python/stub-gen/src/main.rs b/bindings/python/stub-gen/src/main.rs index dbc27d07..726da276 100644 --- a/bindings/python/stub-gen/src/main.rs +++ b/bindings/python/stub-gen/src/main.rs @@ -99,6 +99,7 @@ mod stubs { cwd: typing.Optional[str], env: typing.List[typing.Tuple[str, str]], wait_ready: typing.Optional[bool], + restart: bool, profile_scrollback: typing.Optional[int], profile_colors: typing.List[typing.Tuple[str, str]], text_timeout: typing.Optional[int], @@ -117,6 +118,7 @@ mod stubs { cwd: typing.Optional[str], env: typing.List[typing.Tuple[str, str]], wait_ready: typing.Optional[bool], + restart: bool, profile_scrollback: typing.Optional[int], profile_colors: typing.List[typing.Tuple[str, str]], text_timeout: typing.Optional[int], diff --git a/bindings/python/tests/test_options.py b/bindings/python/tests/test_options.py index 139fa109..62ca6361 100644 --- a/bindings/python/tests/test_options.py +++ b/bindings/python/tests/test_options.py @@ -109,6 +109,7 @@ def test_open_uses_typed_arguments(self): cols=120, rows=40, env={"K": "V"}, + restart=True, profile=Profile( scrollback=321, colors=Colors(red="#010203"), @@ -119,9 +120,10 @@ def test_open_uses_typed_arguments(self): name, args = terminal.fake.calls[0] self.assertEqual(name, "open") self.assertEqual(args[:6], (None, 120, 40, None, [("K", "V")], None)) - self.assertEqual(args[6], 321) - self.assertEqual(args[7], [("red", "#010203")]) - self.assertEqual(args[8:], (100, None, None, None, 200)) + self.assertTrue(args[6]) + self.assertEqual(args[7], 321) + self.assertEqual(args[8], [("red", "#010203")]) + self.assertEqual(args[9:], (100, None, None, None, 200)) def test_run_uses_program_and_argv(self): terminal = _CapturingClient("s") @@ -137,8 +139,9 @@ def test_constructor_profile_is_forwarded_to_run(self): ) run(terminal.run("vim")) args = terminal.fake.calls[0][1] - self.assertIsNone(args[7]) - self.assertEqual(args[8], [("background", "#112233")]) + self.assertFalse(args[7]) + self.assertIsNone(args[8]) + self.assertEqual(args[9], [("background", "#112233")]) def test_input_helpers_use_distinct_typed_methods(self): terminal = _CapturingClient("s") diff --git a/crates/tui-test-cli/src/cli.rs b/crates/tui-test-cli/src/cli.rs index de11597d..b8063d8c 100644 --- a/crates/tui-test-cli/src/cli.rs +++ b/crates/tui-test-cli/src/cli.rs @@ -135,6 +135,9 @@ pub enum Command { /// Return as soon as the shell is spawned, without waiting for a prompt. #[arg(long, conflicts_with = "wait_ready")] no_wait_ready: bool, + /// Replace a live session instead of reusing it. + #[arg(long, visible_alias = "force")] + restart: bool, #[command(flatten)] profile: ProfileArgs, #[command(flatten)] @@ -166,6 +169,9 @@ pub enum Command { /// Return as soon as the program is spawned (the default). #[arg(long, conflicts_with = "wait_ready")] no_wait_ready: bool, + /// Replace a live session instead of reusing it. + #[arg(long, visible_alias = "force")] + restart: bool, #[command(flatten)] profile: ProfileArgs, #[command(flatten)] @@ -374,6 +380,31 @@ mod tests { )); } + #[test] + fn open_and_run_accept_restart_and_force() { + for args in [ + vec!["tui-test", "open", "--restart"], + vec!["tui-test", "open", "--force"], + ] { + let cli = Cli::try_parse_from(args).expect("parse open restart"); + assert!(matches!( + cli.command, + Some(Command::Open { restart: true, .. }) + )); + } + + for args in [ + vec!["tui-test", "run", "--restart", "vim"], + vec!["tui-test", "run", "--force", "vim"], + ] { + let cli = Cli::try_parse_from(args).expect("parse run restart"); + assert!(matches!( + cli.command, + Some(Command::Run { restart: true, .. }) + )); + } + } + #[test] fn open_shell_values_map_to_library_shells() { let cases = [ diff --git a/crates/tui-test-cli/src/config.rs b/crates/tui-test-cli/src/config.rs index a3576656..97d99028 100644 --- a/crates/tui-test-cli/src/config.rs +++ b/crates/tui-test-cli/src/config.rs @@ -21,6 +21,10 @@ pub fn pid_file(session: &str) -> PathBuf { home_dir().join(format!("{session}.pid")) } +pub fn daemon_lock_file(session: &str) -> PathBuf { + home_dir().join(format!("{session}.pid.lock")) +} + pub fn log_file(session: &str) -> PathBuf { home_dir().join(format!("{session}.log")) } diff --git a/crates/tui-test-cli/src/main.rs b/crates/tui-test-cli/src/main.rs index b3c98134..46140397 100644 --- a/crates/tui-test-cli/src/main.rs +++ b/crates/tui-test-cli/src/main.rs @@ -109,7 +109,7 @@ fn connect_to_daemon( let mut last = None; for attempt in 0..ATTEMPTS { if !(allow_incompatible && ipc::is_running(&socket)) { - ensure_daemon(session, verbose) + let _ = ensure_daemon(session, verbose) .map_err(|e| anyhow::anyhow!("failed to start daemon: {e}"))?; } match ipc::connect(&socket) { @@ -142,6 +142,7 @@ fn build_request(command: Command) -> anyhow::Result { env, wait_ready, no_wait_ready, + restart, profile, timeouts, } => Request::Open { @@ -153,6 +154,7 @@ fn build_request(command: Command) -> anyhow::Result { cwd, env: parse_env(&env)?, wait_ready: ready_flag(wait_ready, no_wait_ready), + restart, timeouts: timeouts.into(), }, Command::Run { @@ -164,6 +166,7 @@ fn build_request(command: Command) -> anyhow::Result { env, wait_ready, no_wait_ready, + restart, profile, timeouts, } => { @@ -178,6 +181,7 @@ fn build_request(command: Command) -> anyhow::Result { cwd, env: parse_env(&env)?, wait_ready: ready_flag(wait_ready, no_wait_ready), + restart, timeouts: timeouts.into(), } } @@ -376,62 +380,235 @@ fn parse_env(pairs: &[String]) -> anyhow::Result> { .collect() } -/// Spawn the daemon for this session if it is not already running. -fn ensure_daemon(session: &str, verbose: bool) -> anyhow::Result<()> { - let socket = config::socket_name(session); - match ipc::send(&socket, &Request::Status) { - Ok(status) => { - check_daemon_version(session, &status)?; - if verbose { - eprintln!( - "note: daemon for session '{session}' is already running; verbose logging only \ - applies to a freshly started daemon. Run `tui-test --session {session} close` \ - first, then retry with --verbose." - ); +const DAEMON_STATE_TIMEOUT: Duration = Duration::from_secs(5); +const DAEMON_LOCK_TIMEOUT: Duration = Duration::from_secs(35); +const DAEMON_LOCK_STALE_AFTER: Duration = Duration::from_secs(30); + +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +enum DaemonStart { + AlreadyRunning, + Started, + Restarted, +} + +struct DaemonLock { + path: std::path::PathBuf, +} + +impl DaemonLock { + fn acquire(session: &str) -> anyhow::Result { + Self::acquire_path( + config::daemon_lock_file(session), + DAEMON_LOCK_TIMEOUT, + DAEMON_LOCK_STALE_AFTER, + ) + } + + fn acquire_path( + path: std::path::PathBuf, + timeout: Duration, + stale_after: Duration, + ) -> anyhow::Result { + let start = Instant::now(); + loop { + match std::fs::OpenOptions::new() + .write(true) + .create_new(true) + .open(&path) + { + Ok(_) => return Ok(Self { path }), + Err(error) if error.kind() == std::io::ErrorKind::AlreadyExists => { + if daemon_lock_is_stale(&path, stale_after) { + match std::fs::remove_file(&path) { + Ok(()) => continue, + Err(error) if error.kind() == std::io::ErrorKind::NotFound => continue, + Err(_) => {} + } + } + if start.elapsed() >= timeout { + anyhow::bail!( + "timed out waiting for daemon lifecycle lock {}", + path.display() + ); + } + std::thread::sleep(Duration::from_millis(25)); + } + Err(error) => { + anyhow::bail!( + "failed to acquire daemon lifecycle lock {}: {error}", + path.display() + ); + } } - return Ok(()); } - Err(error) if ipc::is_running(&socket) => anyhow::bail!( - "could not verify the daemon for session '{session}': {error}; run \ - `tui-test --session {session} close`, then retry" - ), - Err(_) => {} } +} + +impl Drop for DaemonLock { + fn drop(&mut self) { + let _ = std::fs::remove_file(&self.path); + } +} + +fn daemon_lock_is_stale(path: &Path, stale_after: Duration) -> bool { + std::fs::metadata(path) + .and_then(|metadata| metadata.modified()) + .ok() + .and_then(|modified| modified.elapsed().ok()) + .is_some_and(|age| age >= stale_after) +} + +/// Spawn or replace the daemon for this session when necessary. +fn ensure_daemon(session: &str, verbose: bool) -> anyhow::Result { + let socket = config::socket_name(session); + if let Some(version) = running_daemon_version(session, &socket)? { + if version == env!("CARGO_PKG_VERSION") { + report_existing_daemon(session, verbose); + return Ok(DaemonStart::AlreadyRunning); + } + } + config::ensure_home()?; - let exe = std::env::current_exe()?; - spawn_detached(&exe, session, verbose)?; + let _lock = DaemonLock::acquire(session)?; - let start = Instant::now(); - while start.elapsed() < Duration::from_secs(5) { - if ipc::is_running(&socket) { - if verbose { - eprintln!("daemon logging to {}", config::log_file(session).display()); - } - return Ok(()); + match running_daemon_version(session, &socket)? { + Some(version) if version == env!("CARGO_PKG_VERSION") => { + report_existing_daemon(session, verbose); + Ok(DaemonStart::AlreadyRunning) } - std::thread::sleep(Duration::from_millis(50)); + Some(version) => { + restart_daemon(session, &socket, &version, verbose)?; + Ok(DaemonStart::Restarted) + } + None => { + start_daemon(session, &socket, verbose)?; + Ok(DaemonStart::Started) + } + } +} + +fn running_daemon_version(session: &str, socket: &str) -> anyhow::Result> { + match ipc::send(socket, &Request::Status) { + Ok(status) => Ok(Some(daemon_version(&status).to_string())), + Err(error) if ipc::is_running(socket) => anyhow::bail!( + "could not verify the daemon for session '{session}': {error}; run \ + `tui-test --session {session} close`, then retry" + ), + Err(_) => Ok(None), } - anyhow::bail!("daemon did not become ready") } -fn check_daemon_version(session: &str, status: &Response) -> anyhow::Result<()> { - let current = env!("CARGO_PKG_VERSION"); - let running = status +fn daemon_version(status: &Response) -> &str { + status .data .as_ref() .and_then(|data| data.get("version")) - .and_then(serde_json::Value::as_str); - if running == Some(current) { - return Ok(()); + .and_then(serde_json::Value::as_str) + .unwrap_or("unknown") +} + +fn report_existing_daemon(session: &str, verbose: bool) { + if verbose { + eprintln!( + "note: daemon for session '{session}' is already running; verbose logging only \ + applies to a freshly started daemon. Run `tui-test --session {session} close` \ + first, then retry with --verbose." + ); } +} - let running = running.unwrap_or("unknown"); - anyhow::bail!( - "daemon for session '{session}' is version {running}, but this client is {current}; \ - run `tui-test --session {session} close` with this client, then retry" +fn restart_daemon( + session: &str, + socket: &str, + running_version: &str, + verbose: bool, +) -> anyhow::Result<()> { + restart_daemon_with( + session, + running_version, + verbose, + || shutdown_daemon(socket), + |expected| wait_for_daemon_state(socket, expected, DAEMON_STATE_TIMEOUT), + || { + let exe = std::env::current_exe()?; + spawn_detached(&exe, session, verbose) + }, ) } +fn restart_daemon_with( + session: &str, + running_version: &str, + verbose: bool, + stop: Stop, + mut wait: Wait, + spawn: Spawn, +) -> anyhow::Result<()> +where + Stop: FnOnce() -> anyhow::Result<()>, + Wait: FnMut(bool) -> anyhow::Result<()>, + Spawn: FnOnce() -> anyhow::Result<()>, +{ + stop().map_err(|error| { + anyhow::anyhow!( + "failed to stop daemon version {running_version} for session '{session}': {error}" + ) + })?; + wait(false) + .map_err(|error| anyhow::anyhow!("daemon for session '{session}' did not stop: {error}"))?; + + if verbose { + eprintln!( + "restarting daemon for session '{session}' from version {running_version} to {}", + env!("CARGO_PKG_VERSION") + ); + } + spawn()?; + wait(true).map_err(|error| anyhow::anyhow!("daemon did not become ready: {error}")) +} + +fn shutdown_daemon(socket: &str) -> anyhow::Result<()> { + let response = ipc::send(socket, &Request::Shutdown)?; + if response.ok { + Ok(()) + } else { + anyhow::bail!( + "{}", + response + .message + .as_deref() + .unwrap_or("daemon refused to stop without an error message") + ) + } +} + +fn start_daemon(session: &str, socket: &str, verbose: bool) -> anyhow::Result<()> { + let exe = std::env::current_exe()?; + spawn_detached(&exe, session, verbose)?; + wait_for_daemon_state(socket, true, DAEMON_STATE_TIMEOUT) + .map_err(|error| anyhow::anyhow!("daemon did not become ready: {error}"))?; + + if verbose { + eprintln!("daemon logging to {}", config::log_file(session).display()); + } + Ok(()) +} + +fn wait_for_daemon_state(socket: &str, expected: bool, timeout: Duration) -> anyhow::Result<()> { + let start = Instant::now(); + while start.elapsed() < timeout { + if ipc::is_running(socket) == expected { + return Ok(()); + } + std::thread::sleep(Duration::from_millis(50)); + } + if expected { + anyhow::bail!("socket never started accepting connections") + } else { + anyhow::bail!("socket kept accepting connections") + } +} + #[cfg(windows)] fn spawn_detached(exe: &Path, session: &str, verbose: bool) -> anyhow::Result<()> { use std::os::windows::process::CommandExt; @@ -561,20 +738,31 @@ fn close_all(json: bool) -> i32 { /// Start a session's daemon, returning only after the socket accepts connections. fn daemon_start(session: &str, verbose: bool, json: bool) -> i32 { - let running = ipc::is_running(&config::socket_name(session)); - if let Err(e) = ensure_daemon(session, verbose) { - eprintln!("failed to start daemon: {e}"); - return 4; - } + let outcome = match ensure_daemon(session, verbose) { + Ok(outcome) => outcome, + Err(e) => { + eprintln!("failed to start daemon: {e}"); + return 4; + } + }; if json { println!( "{}", - serde_json::json!({ "ok": true, "session": session, "started": !running }) + serde_json::json!({ + "ok": true, + "session": session, + "started": outcome != DaemonStart::AlreadyRunning, + "restarted": outcome == DaemonStart::Restarted, + }) ); - } else if running { - println!("daemon already running for session '{session}'"); } else { - println!("started daemon for session '{session}'"); + match outcome { + DaemonStart::AlreadyRunning => { + println!("daemon already running for session '{session}'"); + } + DaemonStart::Started => println!("started daemon for session '{session}'"), + DaemonStart::Restarted => println!("restarted daemon for session '{session}'"), + } } 0 } @@ -709,8 +897,8 @@ fn usage_text() -> &'static str { "tui-test: headless terminal cli + daemon\n\ \n\ SESSION open [--shell S] [--cols N --rows N] [--cwd D] [--env K=V]\n\ - [--config F] [--profile P]\n\ - run [--config F] [--profile P] [args...]\n\ + [--config F] [--profile P] [--restart]\n\ + run [--config F] [--profile P] [--restart] [args...]\n\ sessions | close [--all] | daemon start|status | daemon stop --session N|--all\n\ INSPECT state | text [--full] | screenshot [-o file.svg] [--full]\n\ cells X Y [W H] | get command|output|exit-code|cwd|cursor|size|title\n\ @@ -746,6 +934,15 @@ fn exit_code(resp: &Response) -> i32 { mod tests { use super::*; use serde_json::json; + use std::cell::RefCell; + + fn unique_test_path(label: &str) -> std::path::PathBuf { + let nonce = std::time::SystemTime::now() + .duration_since(std::time::UNIX_EPOCH) + .unwrap() + .as_nanos(); + std::env::temp_dir().join(format!("tui-test-{label}-{}-{nonce}", std::process::id())) + } #[test] fn a_text_only_payload_prints_the_bare_screen() { @@ -784,21 +981,79 @@ mod tests { } #[test] - fn daemon_version_check_rejects_stale_or_unversioned_daemons() { + fn daemon_version_identifies_stale_or_unversioned_daemons() { let current = Response::with(json!({ "version": env!("CARGO_PKG_VERSION") })); - assert!(check_daemon_version("work", ¤t).is_ok()); - - for status in [ - Response::with(json!({ "version": "0.0.0-old" })), - Response::with(json!({})), - ] { - let error = check_daemon_version("work", &status) - .unwrap_err() - .to_string(); - assert!( - error.contains("tui-test --session work close"), - "the recovery instruction is actionable: {error}" - ); - } + assert_eq!(daemon_version(¤t), env!("CARGO_PKG_VERSION")); + + let stale = Response::with(json!({ "version": "0.0.0-old" })); + assert_eq!(daemon_version(&stale), "0.0.0-old"); + + let unversioned = Response::with(json!({})); + assert_eq!(daemon_version(&unversioned), "unknown"); + } + + #[test] + fn daemon_lifecycle_lock_serializes_and_recovers_stale_files() { + let root = unique_test_path("daemon-lock"); + std::fs::create_dir_all(&root).unwrap(); + let path = root.join("work.pid.lock"); + let first = DaemonLock::acquire_path( + path.clone(), + Duration::from_secs(1), + Duration::from_secs(30), + ) + .unwrap(); + + let blocked_path = path.clone(); + let blocked = std::thread::spawn(move || { + let start = Instant::now(); + let lock = DaemonLock::acquire_path( + blocked_path, + Duration::from_secs(1), + Duration::from_secs(30), + ) + .unwrap(); + (start.elapsed(), lock) + }); + std::thread::sleep(Duration::from_millis(100)); + drop(first); + let (waited, second) = blocked.join().unwrap(); + assert!(waited >= Duration::from_millis(75)); + drop(second); + + std::fs::write(&path, b"stale").unwrap(); + let recovered = + DaemonLock::acquire_path(path.clone(), Duration::from_secs(1), Duration::ZERO).unwrap(); + drop(recovered); + let _ = std::fs::remove_dir_all(root); + } + + #[test] + fn daemon_restart_waits_for_shutdown_before_spawning() { + let events = RefCell::new(Vec::new()); + restart_daemon_with( + "work", + "0.0.0-old", + false, + || { + events.borrow_mut().push("shutdown"); + Ok(()) + }, + |expected| { + events + .borrow_mut() + .push(if expected { "started" } else { "stopped" }); + Ok(()) + }, + || { + events.borrow_mut().push("spawn"); + Ok(()) + }, + ) + .unwrap(); + assert_eq!( + events.into_inner(), + ["shutdown", "stopped", "spawn", "started"] + ); } } diff --git a/crates/tui-test-cli/src/protocol.rs b/crates/tui-test-cli/src/protocol.rs index 4bac1daf..d357f3ca 100644 --- a/crates/tui-test-cli/src/protocol.rs +++ b/crates/tui-test-cli/src/protocol.rs @@ -27,6 +27,8 @@ pub enum Request { #[serde(default)] wait_ready: Option, #[serde(default)] + restart: bool, + #[serde(default)] timeouts: Timeouts, }, Close, @@ -160,6 +162,7 @@ impl Request { cwd, env, wait_ready, + restart, timeouts, } => { if let Some(program) = program { @@ -176,6 +179,7 @@ impl Request { cwd, env, wait_ready, + restart, timeouts, })) } else { @@ -187,6 +191,7 @@ impl Request { cwd, env, wait_ready, + restart, timeouts, })) } @@ -399,6 +404,7 @@ mod tests { cwd: None, env: vec![], wait_ready, + restart: false, timeouts, } } @@ -412,11 +418,13 @@ mod tests { Request::Open { wait_ready, cols, + restart, timeouts, .. } => { assert_eq!(wait_ready, None); assert_eq!(cols, 80); + assert!(!restart); assert_eq!(timeouts, Timeouts::default()); } other => panic!("expected Open, got {other:?}"), diff --git a/crates/tui-test-cli/tests/session_lifecycle.rs b/crates/tui-test-cli/tests/session_lifecycle.rs index cc73e1f9..2c81c636 100644 --- a/crates/tui-test-cli/tests/session_lifecycle.rs +++ b/crates/tui-test-cli/tests/session_lifecycle.rs @@ -3,6 +3,7 @@ use std::path::PathBuf; use std::process::{Command, Output}; use std::sync::atomic::{AtomicU32, Ordering}; +use std::sync::{Arc, Barrier}; use std::time::{Duration, Instant}; const BIN: &str = env!("CARGO_BIN_EXE_tui-test"); @@ -182,6 +183,33 @@ fn close_is_idempotent() { sandbox.ok(&["close"]); } +#[test] +fn open_reuses_a_live_child_unless_restart_is_requested() { + let sandbox = Sandbox::new("open-reuse"); + let first = sandbox.ok(&["--json", "open"]); + let first: serde_json::Value = serde_json::from_str(&first).expect("first open json"); + let first_pid = first["data"]["shell_pid"] + .as_u64() + .expect("first open reports a child pid"); + + let reused = sandbox.ok(&["--json", "open"]); + let reused: serde_json::Value = serde_json::from_str(&reused).expect("reused open json"); + assert_eq!( + reused["data"]["shell_pid"].as_u64(), + Some(first_pid), + "a second open should attach to the live child" + ); + + let restarted = sandbox.ok(&["--json", "open", "--restart"]); + let restarted: serde_json::Value = + serde_json::from_str(&restarted).expect("restarted open json"); + assert_ne!( + restarted["data"]["shell_pid"].as_u64(), + Some(first_pid), + "--restart should replace the live child" + ); +} + #[test] fn wait_ready_succeeds_on_an_open_shell() { let sandbox = Sandbox::new("ready"); @@ -977,6 +1005,42 @@ fn daemon_start_is_idempotent_and_makes_status_answer() { sandbox.ok(&["daemon", "stop"]); } +#[test] +fn concurrent_daemon_starts_are_serialized() { + let sandbox = Sandbox::new("start-race"); + let barrier = Arc::new(Barrier::new(3)); + let workers: Vec<_> = (0..2) + .map(|_| { + let barrier = Arc::clone(&barrier); + let home = sandbox.home.clone(); + let session = sandbox.session.clone(); + std::thread::spawn(move || { + barrier.wait(); + Command::new(BIN) + .args(["--session", &session, "--json", "daemon", "start"]) + .env("TUI_TEST_HOME", home) + .output() + .expect("spawn concurrent daemon start") + }) + }) + .collect(); + barrier.wait(); + + let mut started = 0; + for worker in workers { + let output = worker.join().unwrap(); + assert!( + output.status.success(), + "concurrent start failed: {}", + String::from_utf8_lossy(&output.stderr) + ); + let payload: serde_json::Value = + serde_json::from_slice(&output.stdout).expect("daemon start json"); + started += usize::from(payload["started"].as_bool() == Some(true)); + } + assert_eq!(started, 1, "exactly one client should spawn the daemon"); +} + #[test] fn daemon_start_leaves_the_socket_ready() { let sandbox = Sandbox::new("start-ready"); diff --git a/crates/tui-test/src/api.rs b/crates/tui-test/src/api.rs index ba295888..53a7d476 100644 --- a/crates/tui-test/src/api.rs +++ b/crates/tui-test/src/api.rs @@ -40,6 +40,7 @@ pub struct OpenOptions { pub cwd: Option, pub env: Vec<(String, String)>, pub wait_ready: Option, + pub restart: bool, pub timeouts: Timeouts, } @@ -53,6 +54,7 @@ impl Default for OpenOptions { cwd: None, env: Vec::new(), wait_ready: None, + restart: false, timeouts: Timeouts::default(), } } @@ -72,6 +74,7 @@ pub struct RunOptions { pub cwd: Option, pub env: Vec<(String, String)>, pub wait_ready: Option, + pub restart: bool, pub timeouts: Timeouts, } diff --git a/crates/tui-test/src/engine.rs b/crates/tui-test/src/engine.rs index e327359e..7cf42f93 100644 --- a/crates/tui-test/src/engine.rs +++ b/crates/tui-test/src/engine.rs @@ -53,18 +53,19 @@ pub struct LiveFrame { fn operation_summary(operation: &Operation) -> String { match operation { Operation::Open(options) => format!( - "Open {{ shell: {:?}, scrollback: {}, {}x{}, cwd: {:?}, wait_ready: {:?}, timeouts: {:?}, env: <{} vars> }}", + "Open {{ shell: {:?}, scrollback: {}, {}x{}, cwd: {:?}, wait_ready: {:?}, restart: {}, timeouts: {:?}, env: <{} vars> }}", options.shell, options.profile.scrollback, options.cols, options.rows, options.cwd, options.wait_ready, + options.restart, options.timeouts, options.env.len() ), Operation::Run(options) => format!( - "Run {{ program: {:?}, args: {:?}, scrollback: {}, {}x{}, cwd: {:?}, wait_ready: {:?}, timeouts: {:?}, env: <{} vars> }}", + "Run {{ program: {:?}, args: {:?}, scrollback: {}, {}x{}, cwd: {:?}, wait_ready: {:?}, restart: {}, timeouts: {:?}, env: <{} vars> }}", options.program, options.args, options.profile.scrollback, @@ -72,6 +73,7 @@ fn operation_summary(operation: &Operation) -> String { options.rows, options.cwd, options.wait_ready, + options.restart, options.timeouts, options.env.len() ), @@ -144,6 +146,7 @@ impl Engine { options.cwd, options.env, options.wait_ready, + options.restart, options.timeouts, ) } @@ -161,6 +164,7 @@ impl Engine { options.cwd, options.env, options.wait_ready, + options.restart, options.timeouts, ) } @@ -176,8 +180,21 @@ impl Engine { cwd: Option, env: Vec<(String, String)>, wait_ready: Option, + restart: bool, timeouts: crate::api::Timeouts, ) -> Result { + let mut current = self.lock_session(); + if let Some(previous) = current.as_ref() { + if previous.is_alive() && !restart { + return Ok(OpenResult { + shell_pid: previous.pid(), + session: self.name.clone(), + ready: previous.is_ready(), + recording: self.recording_path.to_string_lossy().into_owned(), + }); + } + } + *self .live .lock() @@ -186,9 +203,10 @@ impl Engine { .interrupt .lock() .unwrap_or_else(std::sync::PoisonError::into_inner) = None; - if let Some(previous) = self.lock_session().take() { + if let Some(previous) = current.take() { previous.kill(); } + drop(current); let session = TerminalSession::open( shell, program.clone(), diff --git a/crates/tui-test/src/session.rs b/crates/tui-test/src/session.rs index 3565f603..e7cb5968 100644 --- a/crates/tui-test/src/session.rs +++ b/crates/tui-test/src/session.rs @@ -224,4 +224,40 @@ impl Session { .unwrap_or_else(std::sync::PoisonError::into_inner) .pid() } + + pub fn is_alive(&self) -> bool { + if self + .state + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .exited + .is_some() + { + return false; + } + + let exit_code = self + .pty + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .try_wait(); + let Some(exit_code) = exit_code else { + return true; + }; + + let mut state = self + .state + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner); + state.exited.get_or_insert(exit_code); + false + } + + pub fn is_ready(&self) -> bool { + self.state + .lock() + .unwrap_or_else(std::sync::PoisonError::into_inner) + .tracker + .is_ready() + } } diff --git a/crates/tui-test/tests/runtime.rs b/crates/tui-test/tests/runtime.rs index ed7f2b14..4215e7b4 100644 --- a/crates/tui-test/tests/runtime.rs +++ b/crates/tui-test/tests/runtime.rs @@ -55,6 +55,36 @@ fn named_handles_share_a_process_local_terminal() { .expect("close replacement through first handle"); } +#[test] +fn opening_a_live_named_session_reuses_it_unless_restart_is_requested() { + let registry = SessionRegistry::default(); + let session = registry.session("native-reuse"); + + let first = session + .open(OpenOptions { + wait_ready: Some(false), + ..OpenOptions::default() + }) + .expect("open first terminal"); + let reused = session + .open(OpenOptions { + wait_ready: Some(false), + ..OpenOptions::default() + }) + .expect("reuse live terminal"); + assert_eq!(reused.shell_pid, first.shell_pid); + + let restarted = session + .open(OpenOptions { + wait_ready: Some(false), + restart: true, + ..OpenOptions::default() + }) + .expect("restart live terminal"); + assert_ne!(restarted.shell_pid, first.shell_pid); + session.close().expect("close restarted terminal"); +} + #[test] fn unrelated_session_state_does_not_wait_behind_another_session() { let registry = Arc::new(SessionRegistry::default());