diff --git a/Cargo.toml b/Cargo.toml index 959183d..56d6f7b 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -3,7 +3,7 @@ authors = ["Christopher Sardegna "] categories = ["command-line-interface", "command-line-utilities"] description = "A fleet-view supervisor for arbitrary shell commands." edition = "2024" -exclude = [".github", "docs", ".gitignore", ".gitattributes"] +exclude = [".github", "docs", "tests", ".gitignore", ".gitattributes"] keywords = ["cli", "tui", "pty", "supervisor", "process"] license = "GPL-3.0-or-later" name = "fleetcom" diff --git a/docs/commands.md b/docs/commands.md index f6649ae..98a0b39 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -21,6 +21,7 @@ | Key | Command | | -- | -- | | `↑` `↓` / `k` `j` | Move the selection | +| `Tab` / `Shift-Tab` | Jump the selection to the next / previous section | | `Enter` | Attach to the selected task | | `Space` | Peek at the selected task | | `n` | New command in the invocation directory | @@ -113,7 +114,7 @@ The daemon removes control characters, trims surrounding whitespace, and limits - Recent directories: ones you've launched in before; `Enter` runs there, `Tab`/`→` browses into them. - Subdirectories of the current path: `Enter` or `Tab`/`→` descends into one. -Typing filters the rows; `Backspace` climbs the typed path; `↑`/`↓` move the highlight; `Esc` cancels. Completion updates on each input, permitting navigation and launch without leaving the dashboard. `←`/`→` move the caret within the typed path (`→` descends only when the caret is at the end), and `Ctrl-A`/`Ctrl-E` (or `Home`/`End`) jump to either end; the same caret keys work in every `fleetcom` text field. +Typing filters the rows; `Backspace` deletes one character and the matches re-filter; `↑`/`↓` move the highlight; `Esc` cancels. Completion updates on each input, permitting navigation and launch without leaving the dashboard. `←`/`→` move the caret within the typed path (`→` descends only when the caret is at the end), and `Ctrl-A`/`Ctrl-E` (or `Home`/`End`) jump to either end; the same caret keys work in every `fleetcom` text field. ## The `g` group picker diff --git a/docs/sessions.md b/docs/sessions.md index 8b84f3c..45a9fd4 100644 --- a/docs/sessions.md +++ b/docs/sessions.md @@ -14,17 +14,18 @@ A session is a launch recipe, not a process snapshot. It records commands, worki The first save creates the directory. This matches the [configuration path resolution](README.md#config-directory-sessions) used by save, list, and load. -The filename derives from the session name. `fleetcom` trims leading and trailing whitespace, replaces control characters and any of `* " / \ < > : | ? .` with `_`, and caps the result at 255 characters: `my/session` becomes `my_session.json`, and `a.b` becomes `a_b.json`. Replacing `.` prevents the session name from supplying another extension. +The filename derives from the session name. `fleetcom` trims leading and trailing whitespace, replaces control characters and any of `* " / \ < > : | ? .` with `_`, and limits the sanitized stem to 250 UTF-8 bytes so the `.json` filename fits within 255 bytes. The cap falls on a character boundary. `my/session` becomes `my_session.json`, and `a.b` becomes `a_b.json`. Replacing `.` prevents the session name from supplying another extension. ## Format -A session file is a JSON object with two fields. `name` holds the session name as typed, trimmed but not sanitized. `dirs` maps each working directory to an ordered list of entries. An entry with neither a group nor a name is a command string. An entry carrying either is an object with `cmd` plus the optional `group` and `name` fields: +A session file is a JSON object with three fields. `version` is the format version, currently 1. `name` holds the session name as typed, trimmed but not sanitized. `dirs` maps each working directory to an ordered list of entries. An entry with neither a group nor a name is a command string. An entry carrying either is an object with `cmd` plus the optional `group` and `name` fields: ```json { + "version": 1, "name": "work/api", "dirs": { - "/home/you/work/api": [ + "~/work/api": [ "cargo watch -x test", { "cmd": "cargo run", "group": "api", "name": "api server" } ], @@ -36,11 +37,13 @@ A session file is a JSON object with two fields. `name` holds the session name a ``` - `name` exists because sanitization collapses distinct session names onto one filename: `a/b` and `a.b` both save to `a_b.json`. Saving compares the stored name against the incoming one and refuses a mismatch with an error naming both sessions. The load picker also displays it, so the list shows `a/b`, not `a_b`. -- Keys under `dirs` are directory paths: each task's working directory. +- Keys under `dirs` are directory paths: each task's working directory. Saves write directories under `$HOME` as `~/...`; other paths stay absolute. On load, `~` expands to `$HOME`, and a relative key resolves against the invocation directory of the client loading the session. - Values are ordered lists. A string member is a bare shell command; the object form adds the optional group and display name assigned on load. Order is preserved, and each command runs in its own PTY under that directory. - Directories serialize alphabetically. Command order remains stable within each directory. -There is no version field; the shape discriminates the schema. An object-valued `dirs` marks the wrapped form shown above. The loader also accepts a flat map whose top-level keys are directories and whose values are entry arrays. In that form, an array-valued key named `dirs` remains a directory entry. Flat-map files list by filename stem because they have no stored name. Saving one writes the wrapped form and permits overwriting it without a stored-name collision check. +The `version` field must be an integer from 1 through the newest format supported by the running `fleetcom`. A missing field means version 1. Invalid or unsupported versions fail to load, and the error reports the file's value and the supported version. + +The shape, not the version, discriminates the schema. An object-valued `dirs` marks the wrapped form shown above. The loader also accepts a flat map whose top-level keys are directories and whose values are entry arrays. In that form, an array-valued key named `dirs` remains a directory entry, but a top-level `version` member is always the format version, never a directory. Flat-map files list by filename stem because they have no stored name. Saving one writes the wrapped form and permits overwriting it without a stored-name collision check. Saves are atomic: `fleetcom` writes and syncs a private temporary file in the session directory, then renames it over the recipe. Recipes persist full command lines, which can embed secrets. New session directories use mode 0700, saves remove group and other permissions from existing session directories, and recipe files use mode 0600. @@ -52,9 +55,10 @@ A bare agent command does not identify its conversation, so saving it verbatim w ```json { + "version": 1, "name": "agents", "dirs": { - "/home/you/work/api": [ + "~/work/api": [ "claude --resume 'c8c4a5cc-0b32-4ba0-a6b4-6ed08c218e0d'", { "cmd": "codex resume '019f5453-de22-7240-b2e5-0d32692aa6d9'", "name": "reviewer" } ] diff --git a/src/app.rs b/src/app.rs index 115af38..afb9bdd 100644 --- a/src/app.rs +++ b/src/app.rs @@ -257,7 +257,7 @@ impl App { /// *this* client's env. The core lives in `fleetcom --daemon`, reached over /// the socket. pub fn connect(rows: u16, cols: u16) -> io::Result { - let stream = crate::daemon::connect_ready()?; + let (stream, origin) = crate::daemon::connect_ready()?; // Split the stream here (the fallible part) so the transport factory in // `assemble` (which owns the wake sender) stays infallible. let read = stream.try_clone()?; @@ -265,6 +265,9 @@ impl App { Box::new(SocketTransport::from_halves(stream, read, wait_tx)) }); app.daemon_backed = true; + // Report when a running daemon could not apply this invocation's + // startup-only scrollback setting. + app.status = crate::daemon::ignored_scrollback_notice(origin); Ok(app) } diff --git a/src/daemon.rs b/src/daemon.rs index 77d438f..948213e 100644 --- a/src/daemon.rs +++ b/src/daemon.rs @@ -33,7 +33,7 @@ use std::{ mpsc::channel, }, thread, - time::Duration, + time::{Duration, Instant}, }; use nix::{ @@ -200,11 +200,37 @@ fn check_hello_ack(kind: u8, payload: &[u8]) -> io::Result<()> { } } +/// Whether this invocation reused a daemon or autostarted one. +#[derive(Clone, Copy, PartialEq, Eq, Debug)] +pub enum DaemonOrigin { + /// The initial connection attempt succeeded. + AlreadyRunning, + /// The connection succeeded after autostarting the daemon. + Autostarted, +} + +/// Return a notice when a running daemon cannot apply a startup-only +/// `--scrollback` value. +fn scrollback_notice(origin: DaemonOrigin, flag: Option) -> Option { + match (origin, flag) { + (DaemonOrigin::AlreadyRunning, Some(lines)) => Some(format!( + "--scrollback {lines} ignored: the daemon was already running and \ + keeps its scrollback until 'fleetcom --kill'" + )), + _ => None, + } +} + +/// Return the scrollback notice for this process's parsed flag. +pub fn ignored_scrollback_notice(origin: DaemonOrigin) -> Option { + scrollback_notice(origin, supervisor::scrollback_flag()) +} + /// Connect (autostarting if needed) and complete the hello handshake: send /// this process's protocol version and launch context, require the daemon's -/// ack. Every launch this connection makes then runs under *this* client's -/// env, and a version mismatch surfaces as one actionable error here instead -/// of a silently wrong environment later. +/// ack. Every launch this connection makes runs under *this* client's env. +/// Returns the daemon origin so callers can report ignored startup-only +/// options. /// /// The daemon serves one client at a time, so a slow handshake means "queued /// behind another client", not failure: announce it and wait without a @@ -214,8 +240,8 @@ fn check_hello_ack(kind: u8, payload: &[u8]) -> io::Result<()> { /// timed-out partial `write_all` would corrupt the framing. Callers run this /// *before* touching terminal state (raw mode, alternate screen), so the /// notice prints normally and Ctrl-C aborts cleanly while waiting. -pub fn connect_ready() -> io::Result { - let mut stream = connect_or_autostart()?; +pub fn connect_ready() -> io::Result<(UnixStream, DaemonOrigin)> { + let (mut stream, origin) = connect_or_autostart()?; let done = Arc::new(AtomicBool::new(false)); { @@ -237,7 +263,7 @@ pub fn connect_ready() -> io::Result { done.store(true, Ordering::Relaxed); let (kind, payload) = reply.map_err(hello_read_error)?; check_hello_ack(kind, &payload)?; - Ok(stream) + Ok((stream, origin)) } /// Convert handshake timeouts to a busy-daemon error; preserve other errors. @@ -260,7 +286,8 @@ fn busy_daemon_error(e: io::Error) -> io::Error { /// connection) must not wedge the UI either. A timed-out write drops the /// connection, so a partial frame is never read. pub fn connect_ready_bounded() -> io::Result { - let mut stream = connect_or_autostart()?; + // Scrollback notices apply only to the initial connection. + let (mut stream, _) = connect_or_autostart()?; stream.set_write_timeout(Some(HANDSHAKE_TIMEOUT))?; let (kind, payload) = encode_hello(&LaunchContext::here()); write_frame(&mut stream, kind, &payload).map_err(busy_daemon_error)?; @@ -273,18 +300,18 @@ pub fn connect_ready_bounded() -> io::Result { } /// Connect to the daemon or start one, then wait up to one second for its socket. -fn connect_or_autostart() -> io::Result { +fn connect_or_autostart() -> io::Result<(UnixStream, DaemonOrigin)> { connect_or_autostart_in(&runtime_dir()) } -/// Validate `dir` before the first daemon connection attempt. -fn connect_or_autostart_in(dir: &Path) -> io::Result { +/// Validate `dir`, connect or autostart, and report which path succeeded. +fn connect_or_autostart_in(dir: &Path) -> io::Result<(UnixStream, DaemonOrigin)> { // Validate before connecting because the hello sends the client's // environment and a successful connection skips daemon-side validation. ensure_runtime_dir(dir)?; let path = socket_in(dir); if let Ok(s) = UnixStream::connect(&path) { - return Ok(s); + return Ok((s, DaemonOrigin::AlreadyRunning)); } // Never unlink the socket here: `ECONNREFUSED` on AF_UNIX can also mean a // live daemon's accept backlog is momentarily full. @@ -293,7 +320,7 @@ fn connect_or_autostart_in(dir: &Path) -> io::Result { spawn_daemon()?; for _ in 0..100 { if let Ok(s) = UnixStream::connect(&path) { - return Ok(s); + return Ok((s, DaemonOrigin::Autostarted)); } thread::sleep(Duration::from_millis(10)); } @@ -364,7 +391,8 @@ pub fn run_kill() -> io::Result<()> { file.read_to_string(&mut pid_str)?; let Some(pid) = pid_str.trim().parse::().ok().filter(|p| *p > 0) else { // Without a usable pid, fall back to a Shutdown frame over the socket. - // This path waits until any attached client disconnects. + // Bound the fallback because an attached client can keep the daemon + // from accepting this connection. return kill_via_socket(); }; @@ -391,22 +419,48 @@ pub fn run_kill() -> io::Result<()> { )) } +/// Timeout applied to blocking socket-fallback kill operations. +const KILL_SOCKET_TIMEOUT: Duration = Duration::from_secs(10); + +/// Timeout reported when the socket-fallback kill exchange does not finish. +fn kill_handshake_timeout() -> io::Error { + io::Error::new( + ErrorKind::TimedOut, + "the daemon is running but did not complete the kill handshake in \ + time (another client may be attached); retry after it detaches, or \ + send SIGTERM to the daemon process directly", + ) +} + +/// Set the read timeout to the remaining deadline budget. +fn arm_read_deadline(s: &UnixStream, deadline: Instant) -> io::Result<()> { + let left = deadline.saturating_duration_since(Instant::now()); + if left.is_zero() { + return Err(kill_handshake_timeout()); + } + s.set_read_timeout(Some(left)) +} + +/// Convert either platform representation of a socket timeout into the +/// kill-handshake timeout. +fn deadline_mapped(e: io::Error) -> io::Error { + if is_timeout(&e) { + kill_handshake_timeout() + } else { + e + } +} + /// Send `Shutdown` when the lock file has no usable pid. Complete the handshake /// first, then wait for the daemon to close the socket after stopping its tasks. fn kill_via_socket() -> io::Result<()> { - let path = socket_path(); - match UnixStream::connect(&path) { - Ok(mut s) => { - let (kind, payload) = encode_hello(&LaunchContext::here()); - write_frame(&mut s, kind, &payload)?; - let (kind, payload) = read_frame(&mut s)?; - check_hello_ack(kind, &payload)?; - let (kind, payload) = encode_command(&Command::Shutdown); - write_frame(&mut s, kind, &payload)?; - let mut buf = [0u8; 256]; - while s.read(&mut buf).map(|n| n > 0).unwrap_or(false) {} - Ok(()) - } + kill_via_socket_at(&socket_path(), KILL_SOCKET_TIMEOUT) +} + +/// Run the socket-fallback kill exchange at `path` using `budget` for I/O. +fn kill_via_socket_at(path: &Path, budget: Duration) -> io::Result<()> { + match UnixStream::connect(path) { + Ok(mut s) => kill_over_stream(&mut s, budget), Err(_) => { eprintln!("fleetcom: no daemon running"); Ok(()) @@ -414,6 +468,55 @@ fn kill_via_socket() -> io::Result<()> { } } +/// Drive the Shutdown exchange with bounded writes and a shared read deadline. +fn kill_over_stream(s: &mut UnixStream, budget: Duration) -> io::Result<()> { + let (kind, payload) = encode_hello(&LaunchContext::here()); + kill_exchange(s, budget, kind, &payload) +} + +/// Drive the bounded Shutdown exchange with a pre-encoded hello frame. +fn kill_exchange( + s: &mut UnixStream, + budget: Duration, + hello_kind: u8, + hello_payload: &[u8], +) -> io::Result<()> { + let deadline = Instant::now() + budget; + s.set_write_timeout(Some(budget))?; + + write_frame(s, hello_kind, hello_payload).map_err(deadline_mapped)?; + arm_read_deadline(s, deadline)?; + let (kind, payload) = read_frame(s).map_err(deadline_mapped)?; + check_hello_ack(kind, &payload)?; + + let (kind, payload) = encode_command(&Command::Shutdown); + write_frame(s, kind, &payload).map_err(deadline_mapped)?; + // Socket closure signals completion. Re-arm each read with the remaining + // budget so the loop cannot outlive the deadline. + let mut buf = [0u8; 256]; + loop { + arm_read_deadline(s, deadline)?; + match s.read(&mut buf) { + Ok(0) => return Ok(()), + Ok(_) => {} + Err(e) if is_timeout(&e) => return Err(kill_handshake_timeout()), + // Retry interrupted reads; the deadline still bounds the loop. + Err(e) if e.kind() == ErrorKind::Interrupted => {} + // The daemon closing mid-drain is completion, same as `Ok(0)`. + Err(e) + if matches!( + e.kind(), + ErrorKind::ConnectionReset | ErrorKind::BrokenPipe | ErrorKind::UnexpectedEof + ) => + { + return Ok(()); + } + // Propagate other errors because they do not confirm daemon exit. + Err(e) => return Err(e), + } + } +} + /// The daemon entry point (`fleetcom --daemon`). Binds the socket and serves clients /// until an explicit shutdown. The supervisor is created once and persists across /// reconnects: tasks outlive any single client. @@ -823,6 +926,86 @@ mod tests { let _ = fs::remove_dir_all(&base); } + /// The notice requires both a flag and an already-running daemon. + #[test] + fn scrollback_notice_requires_flag_and_preexisting_daemon() { + assert_eq!( + scrollback_notice(DaemonOrigin::Autostarted, Some(50_000)), + None + ); + assert_eq!(scrollback_notice(DaemonOrigin::Autostarted, None), None); + assert_eq!(scrollback_notice(DaemonOrigin::AlreadyRunning, None), None); + let notice = scrollback_notice(DaemonOrigin::AlreadyRunning, Some(50_000)).unwrap(); + assert!(notice.contains("--scrollback 50000"), "{notice}"); + assert!(notice.contains("--kill"), "{notice}"); + } + + /// A missing hello response times out the kill exchange. + #[test] + fn kill_via_socket_bounds_the_handshake_wait() { + let base = temp("kill_socket_mute"); + fs::create_dir_all(&base).unwrap(); + let sock = base.join("mute.sock"); + // Leave the connection queued in the listener backlog. + let _listener = UnixListener::bind(&sock).unwrap(); + let start = Instant::now(); + let err = kill_via_socket_at(&sock, Duration::from_millis(200)).unwrap_err(); + assert_eq!(err.kind(), ErrorKind::TimedOut); + assert!(err.to_string().contains("kill handshake"), "{err}"); + assert!( + start.elapsed() < Duration::from_secs(5), + "the deadline must fire, not the test's timeout" + ); + let _ = fs::remove_dir_all(&base); + } + + /// A blocked hello write reports the kill-handshake timeout. + #[test] + fn kill_exchange_maps_a_write_timeout() { + let base = temp("kill_socket_bigenv"); + fs::create_dir_all(&base).unwrap(); + let sock = base.join("mute.sock"); + let _listener = UnixListener::bind(&sock).unwrap(); + let mut s = UnixStream::connect(&sock).unwrap(); + // The listener never accepts, so this payload fills the send buffer. + let oversized = vec![0u8; 8 * 1024 * 1024]; + let err = kill_exchange(&mut s, Duration::from_millis(200), 0, &oversized).unwrap_err(); + assert_eq!(err.kind(), ErrorKind::TimedOut); + assert!(err.to_string().contains("kill handshake"), "{err}"); + let _ = fs::remove_dir_all(&base); + } + + /// A daemon that keeps the socket open after Shutdown times out the drain. + #[test] + fn kill_via_socket_bounds_the_drain_wait() { + let base = temp("kill_socket_drain"); + fs::create_dir_all(&base).unwrap(); + let sock = base.join("stuck.sock"); + let listener = UnixListener::bind(&sock).unwrap(); + let server = thread::spawn(move || { + let (mut s, _) = listener.accept().unwrap(); + let _ = read_frame(&mut s); // hello + let (kind, payload) = encode_event(&Event::HelloOk); + let _ = write_frame(&mut s, kind, &payload); + let _ = read_frame(&mut s); // Shutdown, swallowed + // Hold the socket open until the client drops its end. + let _ = s.read(&mut [0u8; 16]); + }); + let err = kill_via_socket_at(&sock, Duration::from_millis(300)).unwrap_err(); + assert_eq!(err.kind(), ErrorKind::TimedOut); + server.join().unwrap(); + let _ = fs::remove_dir_all(&base); + } + + /// A missing socket makes the fallback a no-op. + #[test] + fn kill_via_socket_without_a_socket_is_a_noop() { + let base = temp("kill_socket_absent"); + fs::create_dir_all(&base).unwrap(); + assert!(kill_via_socket_at(&base.join("absent.sock"), Duration::from_millis(100)).is_ok()); + let _ = fs::remove_dir_all(&base); + } + /// Remove group and other read/execute permissions from a valid directory. #[test] fn ensure_runtime_dir_tightens_harmless_bits() { diff --git a/src/harness/assets.rs b/src/harness/assets.rs index c482e4e..0c5ac3f 100644 --- a/src/harness/assets.rs +++ b/src/harness/assets.rs @@ -4,6 +4,9 @@ //! `task--.json` path per task run. The nonce isolates concurrent //! processes and prevents PID reuse from selecting an existing namespace. //! +//! Namespace lifecycle: `Drop` removes this process's namespace, while +//! `install` reaps sibling namespaces whose owner no longer exists. +//! //! Asset contracts: //! - `claude`: `--settings ` layers a `SessionStart` hook //! (`cat > "$FLEETCOM_CAPTURE_FILE"`) over the user's settings. The hook @@ -79,6 +82,49 @@ pub fn runtime_root(override_dir: Option<&Path>) -> Option { dirs::cache_dir().map(|c| c.join("fleetcom").join("run")) } +/// Best-effort removal of namespace directories whose owner no longer exists. +fn reap_dead_namespaces(root: &Path) { + let Ok(entries) = fs::read_dir(root) else { + return; + }; + for entry in entries.flatten() { + // `DirEntry::file_type` does not follow symlinks, so a symlink named + // like a namespace is not a directory here and stays untouched. + if !entry.file_type().is_ok_and(|t| t.is_dir()) { + continue; + } + let name = entry.file_name(); + let Some(pid) = namespace_owner(name.to_str().unwrap_or("")) else { + continue; + }; + if owner_is_dead(pid) { + let _ = fs::remove_dir_all(entry.path()); + } + } +} + +/// Parse `-<12 lowercase hex characters>`. +fn namespace_owner(name: &str) -> Option { + let (pid, nonce) = name.split_once('-')?; + if pid.is_empty() || !pid.bytes().all(|b| b.is_ascii_digit()) { + return None; + } + if nonce.len() != 12 + || !nonce + .bytes() + .all(|b| b.is_ascii_hexdigit() && !b.is_ascii_uppercase()) + { + return None; + } + pid.parse::().ok().filter(|p| *p > 0) +} + +/// Return true only when signal 0 reports that `pid` does not exist. +fn owner_is_dead(pid: i32) -> bool { + use nix::{errno::Errno, sys::signal::kill, unistd::Pid}; + matches!(kill(Pid::from_raw(pid), None), Err(Errno::ESRCH)) +} + /// Capture assets owned by one supervisor process. #[derive(Debug)] pub struct CaptureAssets { @@ -91,8 +137,8 @@ pub struct CaptureAssets { impl CaptureAssets { /// Create `root` and a private `/-` namespace. The /// namespace uses mode `0700`; its Claude settings use `0600`, and its - /// executable Codex notifier uses `0700`. Existing root entries remain - /// unchanged. + /// executable Codex notifier uses `0700`. Dead-owner namespaces are reaped + /// before the new namespace is created; other root entries remain. pub fn install(root: &Path, pid: u32) -> io::Result { fs::DirBuilder::new() .recursive(true) @@ -101,6 +147,8 @@ impl CaptureAssets { // Recursive creation retains a pre-existing directory's permissions. fs::set_permissions(root, fs::Permissions::from_mode(0o700))?; + reap_dead_namespaces(root); + // The first 12 dash-free UUID characters contain 48 random bits; the // UUID version and variant occur later in the string. let nonce: String = super::uuid_v4() @@ -244,11 +292,11 @@ mod tests { let _ = fs::remove_dir_all(&root); } - /// Installation leaves every pre-existing root entry unchanged. + /// Installation retains live-owner namespaces and non-namespace entries. #[test] - fn install_never_deletes_foreign_or_legacy_files() { + fn install_never_deletes_live_owner_namespaces_or_legacy_files() { let root = temp("assets_retain"); - let foreign = root.join("99999-0123456789ab"); + let foreign = root.join("1-0123456789ab"); fs::create_dir_all(&foreign).unwrap(); fs::write(foreign.join("task-1-0.json"), "{}").unwrap(); fs::write(root.join("task-1-0.json"), "{}").unwrap(); @@ -258,7 +306,7 @@ mod tests { let assets = CaptureAssets::install(&root, std::process::id()).unwrap(); assert!( foreign.join("task-1-0.json").exists(), - "another process's capture file must survive" + "a live process's capture file must survive" ); assert!( root.join("task-1-0.json").exists(), @@ -278,7 +326,7 @@ mod tests { let _ = fs::remove_dir_all(&root); } - /// A matching PID prefix does not cause an existing namespace to be reused. + /// A live matching PID retains its namespace and receives a distinct nonce. #[test] fn install_after_pid_reuse_leaves_the_predecessor_namespace_alone() { let root = temp("assets_reuse"); @@ -317,6 +365,66 @@ mod tests { let _ = fs::remove_dir_all(&root); } + /// Spawn and reap a child, then return its inactive PID. + fn dead_pid() -> u32 { + let mut child = Command::new("sh").arg("-c").arg("exit 0").spawn().unwrap(); + let pid = child.id(); + child.wait().unwrap(); + pid + } + + /// Installation removes a dead owner's namespace and its contents. + #[test] + fn install_reaps_a_dead_owner_namespace() { + let root = temp("assets_reap"); + let dead = root.join(format!("{}-0123456789ab", dead_pid())); + fs::create_dir_all(&dead).unwrap(); + fs::write(dead.join("task-1-0.json"), "{}").unwrap(); + + let assets = CaptureAssets::install(&root, std::process::id()).unwrap(); + assert!(!dead.exists(), "a dead owner's namespace must be reaped"); + assert!(assets.claude_settings.exists()); + let _ = fs::remove_dir_all(&root); + } + + /// A namespace-shaped file is not reaped. + #[test] + fn install_keeps_a_file_named_like_a_dead_namespace() { + let root = temp("assets_reap_file"); + fs::create_dir_all(&root).unwrap(); + let decoy = root.join(format!("{}-0123456789ab", dead_pid())); + fs::write(&decoy, "not a namespace").unwrap(); + + let _assets = CaptureAssets::install(&root, std::process::id()).unwrap(); + assert!(decoy.exists(), "a file is never a reap candidate"); + let _ = fs::remove_dir_all(&root); + } + + /// Malformed namespace names are not reaped. + #[test] + fn install_keeps_directories_with_malformed_namespace_names() { + let root = temp("assets_reap_malformed"); + let names = [ + "abc-0123456789ab", // non-numeric pid + "-1-0123456789ab", // negative pid: empty first field + "+42-0123456789ab", // sign prefix is not a decimal digit + "99999999999-0123456789ab", // past i32::MAX + "42-0123456789AB", // uppercase nonce + "42-0123456789a", // 11-char nonce + "42-0123456789abc", // 13-char nonce + "42", // no dash at all + ]; + for name in names { + fs::create_dir_all(root.join(name)).unwrap(); + } + + let _assets = CaptureAssets::install(&root, std::process::id()).unwrap(); + for name in names { + assert!(root.join(name).exists(), "{name:?} must be kept"); + } + let _ = fs::remove_dir_all(&root); + } + /// The notifier overwrites the capture file with its first argument. With /// no capture path or chain, it exits without producing output. #[test] @@ -478,7 +586,7 @@ mod tests { #[test] fn drop_removes_only_the_incarnation_namespace() { let root = temp("assets_drop"); - let sibling = root.join("99999-0123456789ab"); + let sibling = root.join("1-0123456789ab"); fs::create_dir_all(&sibling).unwrap(); fs::write(sibling.join("task-1-0.json"), "{}").unwrap(); diff --git a/src/main.rs b/src/main.rs index 055fd5b..93ce9c4 100644 --- a/src/main.rs +++ b/src/main.rs @@ -36,7 +36,7 @@ mod testutil; pub(crate) use terminal::{ansi, emulator, format, frame, input}; use std::{ - io, + io::{self, IsTerminal}, sync::{ Arc, atomic::{AtomicBool, Ordering}, @@ -153,7 +153,21 @@ fn parse_args(args: &[String]) -> Result { } } -fn main() -> io::Result<()> { +/// Format a fatal error with the program prefix and its `Display` form. +fn error_line(e: impl std::fmt::Display) -> String { + format!("fleetcom: {e}") +} + +fn main() { + // Present propagated errors consistently and exit with failure. + if let Err(e) = run() { + eprintln!("{}", error_line(&e)); + std::process::exit(1); + } +} + +/// Run the program; `main` presents errors returned from this boundary. +fn run() -> io::Result<()> { let args: Vec = std::env::args().skip(1).collect(); let (foreground, session, scrollback) = match parse_args(&args) { Ok(Invocation::Help) => { @@ -174,12 +188,19 @@ fn main() -> io::Result<()> { scrollback, }) => (foreground, session, scrollback), Err(e) => { - eprintln!("fleetcom: {e}"); + eprintln!("{}", error_line(e)); eprintln!("try 'fleetcom --help'"); std::process::exit(2); } }; + // The interactive client requires stdout for terminal frames. Headless + // and informational modes return before this check. + if !io::stdout().is_terminal() { + eprintln!("{}", error_line("stdout is not a terminal")); + std::process::exit(1); + } + // Install the value before constructing or autostarting a supervisor. if let Some(lines) = scrollback { supervisor::set_scrollback_flag(lines); @@ -419,6 +440,20 @@ mod tests { assert!(parse(&["one", "two"]).is_err()); } + /// Fatal error lines use the program prefix and `Display` representation. + #[test] + fn error_line_prefixes_display_form() { + let e = io::Error::new(io::ErrorKind::TimedOut, "daemon did not exit after SIGTERM"); + assert_eq!( + error_line(&e), + "fleetcom: daemon did not exit after SIGTERM" + ); + assert_eq!( + error_line("stdout is not a terminal"), + "fleetcom: stdout is not a terminal" + ); + } + fn contains(haystack: &[u8], needle: &[u8]) -> bool { haystack.windows(needle.len()).any(|w| w == needle) } diff --git a/src/session.rs b/src/session.rs index 636cd11..0735b19 100644 --- a/src/session.rs +++ b/src/session.rs @@ -28,19 +28,37 @@ pub const FLEETCOM_CONFIG_DIR: &str = "FLEETCOM_CONFIG_DIR"; /// Characters replaced with `_` in session filenames. const DISALLOWED: &[char] = &['*', '"', '/', '\\', '<', '>', ':', '|', '?', '.']; -/// Make `name` safe as a bare filename. +/// Sanitized-stem cap that reserves 5 bytes for `.json` in a 255-byte +/// filename component. +const MAX_STEM_BYTES: usize = 250; + +/// Sanitize `name` and limit its UTF-8 encoding without splitting a character. fn sanitize(name: &str) -> String { - name.trim() - .chars() - .map(|c| { - if c.is_control() || DISALLOWED.contains(&c) { - '_' - } else { - c - } - }) - .take(255) - .collect() + let mut out = String::new(); + for c in name.trim().chars() { + let c = if c.is_control() || DISALLOWED.contains(&c) { + '_' + } else { + c + }; + if out.len() + c.len_utf8() > MAX_STEM_BYTES { + break; + } + out.push(c); + } + out +} + +/// Longest prefix of `s` at most `max` bytes long, on a char boundary. +fn prefix_bytes(s: &str, max: usize) -> &str { + if s.len() <= max { + return s; + } + let mut end = max; + while !s.is_char_boundary(end) { + end -= 1; + } + &s[..end] } /// Session-recipe directory: `/sessions`. A caller-supplied @@ -53,8 +71,12 @@ pub fn sessions_dir(root: Option) -> Option { .map(|base| base.join("sessions")) } -/// Serialize `{"name": , "dirs": {...}}`. The stored name lets -/// `save_in` distinguish names that sanitize to the same filename. +/// Session format version written by `to_json` and accepted by `from_json`. +/// Missing versions are interpreted as version 1; unsupported versions fail. +const FORMAT_VERSION: u64 = 1; + +/// Serialize the versioned wrapped schema. The stored name distinguishes +/// names that sanitize to the same filename. fn to_json(name: &str, cfg: &SessionConfig) -> String { let mut dirs = jzon::JsonValue::new_object(); for (dir, entries) in cfg { @@ -79,6 +101,7 @@ fn to_json(name: &str, cfg: &SessionConfig) -> String { let _ = dirs.insert(dir, arr); } let mut obj = jzon::JsonValue::new_object(); + let _ = obj.insert("version", FORMAT_VERSION); let _ = obj.insert("name", name); let _ = obj.insert("dirs", dirs); obj.pretty(2) @@ -87,15 +110,43 @@ fn to_json(name: &str, cfg: &SessionConfig) -> String { /// Parse wrapped and flat schemas, returning the stored name when present. /// A wrapped file has an object-valued `dirs`; flat files have entry arrays at /// the top level, including when a directory is literally named `dirs`. +/// A top-level `version` must be an integer from 1 through [`FORMAT_VERSION`]; +/// a missing version is interpreted as 1. fn from_json(text: &str) -> io::Result<(Option, SessionConfig)> { let parsed = jzon::parse(text).map_err(|e| io::Error::other(e.to_string()))?; - let (name, dirs) = if parsed["dirs"].is_object() { - (parsed["name"].as_str().map(str::to_string), &parsed["dirs"]) + // Validate version metadata before detecting the schema shape. + let version = &parsed["version"]; + if !version.is_null() { + match version.as_u64() { + Some(n) if (1..=FORMAT_VERSION).contains(&n) => {} + Some(n) if n > FORMAT_VERSION => { + return Err(io::Error::other(format!( + "session format version {n} is newer than this fleetcom \ + (supports {FORMAT_VERSION}); load it with a newer build" + ))); + } + // Reject zero, fractional, negative, and non-numeric values. + _ => { + return Err(io::Error::other(format!( + "session format version {} is not one this fleetcom reads \ + (supports {FORMAT_VERSION}); load it with a newer build", + version.dump() + ))); + } + } + } + let (name, dirs, flat) = if parsed["dirs"].is_object() { + let name = parsed["name"].as_str().map(str::to_string); + (name, &parsed["dirs"], false) } else { - (None, &parsed) + (None, &parsed, true) }; let mut cfg = SessionConfig::new(); for (dir, val) in dirs.entries() { + // In a flat file, `version` is metadata beside the directory keys. + if flat && dir == "version" { + continue; + } // Ignore members that match neither supported entry form. let entries = val .members() @@ -172,7 +223,11 @@ pub fn save_in(dir: &Path, name: &str, cfg: &SessionConfig) -> io::Result Duration { } /// Split a task row into its leading, preview, and time cells so the preview -/// can be styled independently. +/// can be styled independently. Each cell is padded to its display-column +/// budget, and the budgets sum to `cols`. fn task_row_parts(v: &TaskView, cols: usize) -> (String, String, String) { let glyph = match v.lifecycle { Lifecycle::Active => "✻", @@ -335,15 +336,13 @@ fn task_row_parts(v: &TaskView, cols: usize) -> (String, String, String) { let tag = if v.tagged { "◆" } else { " " }; let time = rel_time(row_age(v)); let title_w = 26.min(cols / 3); - let title = truncate(display_label(v), title_w); - // prefix(2) glyph+sp(2) tag+sp(2) title(title_w) sp(1) preview(prev_w) sp(1) time - let used = 2 + 2 + 2 + title_w + 1 + 1 + time.chars().count(); + // indent(2) glyph(1) sp(1) tag(1) title(title_w) sp(1) preview(prev_w) sp(1) time + let used = 2 + 1 + 1 + 1 + title_w + 1 + 1 + time.width(); let prev_w = cols.saturating_sub(used); - let preview = truncate(&v.preview.text, prev_w); ( - format!(" {glyph} {tag}{title: String { /// Paint a task row with only its padded preview cell dimmed. fn dim_preview_row(out: &mut impl Write, y: u16, v: &TaskView, cols: usize) -> io::Result<()> { + // Clamp each styled cell to the display columns still available. let (lead, preview, time) = task_row_parts(v, cols); - let display = pad(&format!("{lead}{preview}{time}"), cols); - let mut chars = display.chars(); - let lead: String = chars.by_ref().take(lead.chars().count()).collect(); - let preview: String = chars.by_ref().take(preview.chars().count()).collect(); - let rest: String = chars.collect(); + let lead = truncate(&lead, cols); + let mut rem = cols - lead.width(); + let preview = truncate(&preview, rem); + rem -= preview.width(); queue!( out, MoveTo(0, y), @@ -368,10 +367,20 @@ fn dim_preview_row(out: &mut impl Write, y: u16, v: &TaskView, cols: usize) -> i SetAttribute(Attribute::Dim), Print(preview), SetAttribute(Attribute::Reset), - Print(rest) + Print(pad(&time, rem)) ) } +/// Build a labeled peek-box border exactly `inner_w` display columns wide. +fn peek_top_border(label: &str, inner_w: usize) -> String { + let mut border = format!("─ {} ", truncate(label, inner_w.saturating_sub(4))); + let w = border.width(); + if w < inner_w { + border.extend(std::iter::repeat_n('─', inner_w - w)); + } + border +} + fn render_peek(out: &mut impl Write, app: &App) -> io::Result<()> { let Some(i) = app.selected_task() else { return Ok(()); @@ -394,19 +403,10 @@ fn render_peek(out: &mut impl Write, app: &App) -> io::Result<()> { let start = lines.len().saturating_sub(inner_h); let tail = &lines[start..]; - // Top border with the task's display label inlined. - let mut top_mid = format!( - "─ {} ", - truncate(display_label(v), inner_w.saturating_sub(4)) - ); - let tl = top_mid.chars().count(); - if tl < inner_w { - top_mid.extend(std::iter::repeat_n('─', inner_w - tl)); - } queue!( out, MoveTo(x0 as u16, y0 as u16), - Print(format!("┌{top_mid}┐")) + Print(format!("┌{}┐", peek_top_border(display_label(v), inner_w))) )?; for k in 0..inner_h { @@ -616,13 +616,13 @@ fn render_session_picker(out: &mut impl Write, app: &App) -> io::Result<()> { /// Center `s` in `width` columns (a full-width string, so it overwrites the row). fn center(s: &str, width: usize) -> String { - let len = s.chars().count(); + let len = s.width(); if len >= width { return truncate(s, width); } let mut out = " ".repeat((width - len) / 2); out.push_str(s); - let cur = out.chars().count(); + let cur = out.width(); out.push_str(&" ".repeat(width - cur)); out } @@ -798,6 +798,66 @@ mod tests { ); } + /// Rows remain column-exact with wide and combining glyphs. + #[test] + fn task_row_is_column_exact_for_wide_glyphs() { + let titles = [ + "plain ascii title", + "日本語のタスクタイトルです", // 13 wide chars: fills title_w exactly at 80 cols + "🚀 emoji 🚀 title", + "e\u{0301}e\u{0301} combining", // combining marks are zero-width + ]; + let previews = [ + "build ok", + "ビルド中の😀プレビュー出力がここに続いています", + "", + ]; + for cols in [40usize, 80] { + for title in titles { + for preview in previews { + let mut v = timed_view(Lifecycle::Active, false, None, None); + v.name = Some(title.to_string()); + v.preview = Preview::floor(preview.to_string()); + let row = task_row(&v, cols); + assert_eq!( + row.width(), + cols, + "cols {cols} title {title:?} preview {preview:?}: {row:?}" + ); + assert!(row.ends_with(" 2h"), "time cell lost: {row:?}"); + } + } + } + } + + /// Row-cell display widths depend on `cols`, not their contents. + #[test] + fn task_row_cells_hold_their_column_budgets() { + let cols = 72; + let ascii = timed_view(Lifecycle::Active, false, None, None); + let mut wide = timed_view(Lifecycle::Active, false, None, None); + wide.name = Some("日本語のテスト".into()); + wide.preview = Preview::floor("進捗 50% 😀".into()); + let (al, ap, at) = task_row_parts(&ascii, cols); + let (wl, wp, wt) = task_row_parts(&wide, cols); + assert_eq!(al.width(), wl.width(), "lead width varies with content"); + assert_eq!(ap.width(), wp.width(), "preview width varies with content"); + assert_eq!(at.width(), wt.width(), "time width varies with content"); + assert_eq!(wl.width() + wp.width() + wt.width(), cols); + } + + /// Peek borders remain column-exact for wide and overlong labels. + #[test] + fn peek_top_border_fills_to_inner_width() { + for label in ["cargo test", "日本語のテスト", "🚀 build", "e\u{0301}", ""] { + let b = peek_top_border(label, 40); + assert_eq!(b.width(), 40, "label {label:?}: {b:?}"); + } + // Overlong labels truncate inside the border rather than widening it. + let b = peek_top_border(&"長".repeat(40), 40); + assert_eq!(b.width(), 40, "{b:?}"); + } + /// The peek footer's provenance label composes source, rule, and frozen. #[test] fn preview_provenance_label_shapes() {