From 8c702e320dba96088078f109d3b40de598cf2326 Mon Sep 17 00:00:00 2001 From: Christopher Sardegna Date: Sun, 18 Oct 2026 16:12:26 -0700 Subject: [PATCH 1/3] Enhance session management by adding support for tags and flagship marks in session entries, updating related documentation and tests. --- README.md | 2 +- docs/README.md | 4 +- docs/commands.md | 2 +- docs/sessions.md | 22 ++--- src/protocol.rs | 4 +- src/session.rs | 147 ++++++++++++++++++++++++++++--- src/supervisor.rs | 36 +++++--- src/supervisor_tests.rs | 185 ++++++++++++++++++++++++++++++++++++++-- 8 files changed, 356 insertions(+), 46 deletions(-) diff --git a/README.md b/README.md index 8faa534..f2f08ac 100644 --- a/README.md +++ b/README.md @@ -40,7 +40,7 @@ Managing several long-lived commands across terminal panes is pesky, especially - Run each command in its own PTY and group tasks by state, working directory, or named group. - Keep tasks running under a daemon across client disconnects. -- Save and reload task recipes: directories, commands, group assignments, and display names. +- Save and reload task recipes: directories, commands, group assignments, display names, tags, and the flagship. - Rerun a completed task in place, keeping its identity, group, and name. - Launch `claude`, `codex`, `grok`, and `omp` as managed agents and resume their conversations on rerun and reload. - Recover the current task set from automatic snapshots. diff --git a/docs/README.md b/docs/README.md index 8b8c85d..bbca19a 100644 --- a/docs/README.md +++ b/docs/README.md @@ -150,7 +150,7 @@ Press `w`, type a name, and press `Enter` to save the fleet as a [session](sessi | `/daemon.lock` | `0666 & ~umask` when new; otherwise unchanged | the owning daemon's PID, trustworthy only while its `flock` is held | | `/daemon.log` | `0666 & ~umask` when new; otherwise unchanged | stderr from the autostarted daemon | | [session directory](#config-directory-sessions) | `0700` | saved recipes | -| `/.json` | `0600` | directories, commands, groups, display names | +| `/.json` | `0600` | directories, commands, groups, display names, tags, the flagship | | `/recovery/` | `0700` | [automatic snapshots](sessions.md#recovery) | | `/recovery/.json` | `0600` | one automatic session recipe | | `/-/` | `0700` | [agent hook, notifier, and extension-module assets plus per-run capture payloads](agent-resume.md#capture-state-and-isolation) | @@ -165,7 +165,7 @@ The runtime directory is validated before its contents are used. The path must b Full command lines are persisted in recipes, including any embedded secrets. A token passed as an argument is written to its session file and to every recovery snapshot that captures the task. -The client environment is not persisted. Each client's environment and working directory are sent during the connection handshake and retained in daemon memory. Only directories, commands, group assignments, and display names are stored in session and recovery files. +The client environment is not persisted. Each client's environment and working directory are sent during the connection handshake and retained in daemon memory. Only directories, commands, group assignments, display names, tags, and the flagship are stored in session and recovery files. ### Captured IDs diff --git a/docs/commands.md b/docs/commands.md index 089dea8..efc192e 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -155,7 +155,7 @@ Press `Ctrl-]` from the dashboard, peek, or an attached task to attach to the fl After attaching to the flagship with `Enter`, press `Ctrl-]` to return to the dashboard. -The flagship mark is cleared on task completion or removal and is not saved in sessions. Tags set with `m` are retained after task completion. +The flagship mark is cleared on task completion or removal. Tags set with `m` are retained after task completion. Both marks are saved in [sessions](sessions.md) and recovery snapshots and restored on load; a stored flagship is applied only when no task is currently marked. #### Renaming diff --git a/docs/sessions.md b/docs/sessions.md index 2459064..f7ae736 100644 --- a/docs/sessions.md +++ b/docs/sessions.md @@ -1,6 +1,6 @@ # Sessions -A session is a launch recipe: commands, working directories, and each task's optional group assignment and display name. Loading a session always starts new processes. To keep live processes running across client disconnects, use the [daemon](README.md#storage-paths). +A session is the saved state of the task set as the daemon holds it. Stored: working directories, commands or managed agents with their session IDs, group assignments, display names, "in use" tags, and the flagship mark. Not stored: anything the client owns (the attached or peeked task, the selection, grouping and sort modes, terminal size) and anything derived from the process (lifecycle, output, timings). Loading a session always starts new processes. To keep live processes running across client disconnects, use the [daemon](README.md#storage-paths). ## Storage @@ -20,8 +20,10 @@ The filename is derived from the session name: leading and trailing whitespace i A session file is a JSON object with three fields: `version` for the format version, currently 2; `name` for the typed session name, trimmed but not sanitized; and `dirs` for an ordered list of entries per working directory. Use one of two entry forms, according to the task's launch mode: -- A **literal** entry is a typed command. Use a bare string without labels, or an object with `cmd` and optional `group` and `name` fields. On load, the stored text is run unchanged through your shell: even `"claude"` is a literal command. -- A **managed** entry is an object with `agent`, the registered program word (`claude`, `codex`, `grok`, or `omp`), plus optional `resume`, `group`, and `name` fields. Tasks launched from the Agent page are saved in this form. On load, the agent is launched as managed; with `resume`, the stored session ID is reopened, and without it, a fresh conversation is started. See [Agent session resume](agent-resume.md). +- A **literal** entry is a typed command. Use a bare string without labels or marks, or an object with `cmd` and optional `group`, `name`, `tagged`, and `flagship` fields. On load, the stored text is run unchanged through your shell: even `"claude"` is a literal command. +- A **managed** entry is an object with `agent`, the registered program word (`claude`, `codex`, `grok`, or `omp`), plus optional `resume`, `group`, `name`, `tagged`, and `flagship` fields. Tasks launched from the Agent page are saved in this form. On load, the agent is launched as managed; with `resume`, the stored session ID is reopened, and without it, a fresh conversation is started. See [Agent session resume](agent-resume.md). + +`tagged` and `flagship` are optional booleans on both object forms: with `true`, the task is tagged ["in use"](commands.md#grouping-and-tagging) or made the [flagship](commands.md#the-flagship); with `false`, `null`, or omission, it is unmarked. Each is written only when true. At most one entry per file may carry `flagship`; with a second, the whole load is refused. ```json { @@ -30,8 +32,8 @@ A session file is a JSON object with three fields: `version` for the format vers "dirs": { "~/work/api": [ "cargo watch -x test", - { "cmd": "cargo run", "group": "api", "name": "api server" }, - { "agent": "claude", "resume": "c8c4a5cc-0b32-4ba0-a6b4-6ed08c218e0d", "group": "agents" }, + { "cmd": "cargo run", "group": "api", "name": "api server", "tagged": true }, + { "agent": "claude", "resume": "c8c4a5cc-0b32-4ba0-a6b4-6ed08c218e0d", "group": "agents", "flagship": true }, { "agent": "omp" } ], "/tmp": [ @@ -54,17 +56,17 @@ Saves are atomic: a private temporary file is written and synced in the session The file is plain JSON and practical to edit by hand. Edit `name` to change the stored session identity: a subsequent save under the old name will be refused by the collision check. A readable stored name is still used for collision checks and picker labels when the command body is invalid or the version unsupported. -The entire recipe is validated before any commands are started. The root must be an object, every directory value must be an array, and every entry must be a command string or an object with a string `cmd` or a string `agent`, never both. `agent` must be a supported program word, and `resume`, when present, must be a lowercase `8-4-4-4-12` UUID or `null`. Strings, `null`, or omission are accepted for optional entry `group` and `name` fields and the wrapped session's `name`; other types are rejected. Unknown fields in wrapped metadata and entry objects are ignored. Empty maps, arrays, and strings are valid. With invalid JSON or any malformed field, the whole load is refused; existing tasks and the recipe file are left intact. In schema errors, the quoted directory key, entry number (starting at 1), and offending field are included where applicable. +The entire recipe is validated before any commands are started. The root must be an object, every directory value must be an array, and every entry must be a command string or an object with a string `cmd` or a string `agent`, never both. `agent` must be a supported program word, and `resume`, when present, must be a lowercase `8-4-4-4-12` UUID or `null`. Strings, `null`, or omission are accepted for optional entry `group` and `name` fields and the wrapped session's `name`; booleans, `null`, or omission for `tagged` and `flagship`; other types are rejected. A second `flagship` is rejected, with both entry locations in the error. Unknown fields in wrapped metadata and entry objects are ignored. Empty maps, arrays, and strings are valid. With invalid JSON or any malformed field, the whole load is refused; existing tasks and the recipe file are left intact. In schema errors, the quoted directory key, entry number (starting at 1), and offending field are included where applicable. After validation, group and display names are stripped of control characters and surrounding whitespace and limited to 64 characters. `Unassigned` is treated as no group but is a legal display name. Missing directories, entries past the task limit, and literal commands past the length limit are skipped and counted in the load status. A managed entry whose agent is not found on the connected client's `PATH` is reported on the status line and counted as a failed spawn; the remaining entries are still loaded. Spawn failures are reported separately; tasks already started from a structurally valid recipe are left running. -Use the string form for commands with neither a group nor a name. String and object entries can appear in the same directory array. +Use the string form for commands with no group, no name, and no mark. String and object entries can appear in the same directory array. ## Recovery The current task set is automatically snapshotted under `recovery/` inside the session directory. A separate file is written for each daemon (or `--foreground` core), named for its start time and process ID. After each write, the new file and files whose process IDs are still live are protected; among the remaining files, the nine newest names are retained. A shared recovery directory can therefore contain more than ten snapshots while multiple writers are live. -A snapshot pass is scheduled two seconds after the last command that can change a saved recipe, coalescing a burst of commands. A nonempty recipe is written when its content or destination changed, or when the expected snapshot file is missing. Content changes that no command caused, such as a managed task's newly captured session ID, are also checked every 60 seconds. +A snapshot pass is scheduled two seconds after the last command that can change a saved recipe, coalescing a burst of commands. Marking with `m` or `]` is such a command. The flagship clear on task exit is not a command, but a pass is scheduled for it too. A nonempty recipe is written when its content or destination changed, or when the expected snapshot file is missing. Content changes that no command caused, such as a managed task's newly captured session ID, are also checked every 60 seconds. - No snapshot is written for an empty fleet. After removing every task, the previous snapshot is retained. - Snapshots are left in place after quitting, disconnecting, or `fleetcom --kill`. @@ -77,8 +79,8 @@ As with saved recipes, full command lines are persisted in recovery files, inclu ## Saving and loading -- Save: press `w` in the dashboard, type a name, and press `Enter` to write `.json` with the session name and each task's directory, command or managed agent and session ID, and optional group and name. +- Save: press `w` in the dashboard, type a name, and press `Enter` to write `.json` with the session name and each task's directory, command or managed agent and session ID, optional group and name, "in use" tag, and flagship mark. - Load in-app: `o`, pick from the list, `Enter`. - Load at launch: `fleetcom `. -On load, new processes are always spawned from the stored commands. Live process state is retained by the daemon and never stored in session files. See [Agent session resume](agent-resume.md) for preserving supported agent conversations across relaunch. +On load, new processes are always spawned from the stored commands. Live process state is retained by the daemon and never stored in session files. Load is additive and never disturbs live marks: tags are restored on every loaded task, but a stored flagship is applied only when no task is currently marked. See [Agent session resume](agent-resume.md) for preserving supported agent conversations across relaunch. diff --git a/src/protocol.rs b/src/protocol.rs index 612d481..4576ac8 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -453,8 +453,8 @@ fn num_from>(v: &jzon::JsonValue) -> Option { } /// Decode an optional boolean flag. Use `false` for missing or null values; reject the -/// message for other non-boolean values. -fn bool_flag(v: &jzon::JsonValue) -> Option { +/// message for other non-boolean values. Session entry marks use the same rule. +pub fn bool_flag(v: &jzon::JsonValue) -> Option { if v.is_null() { return Some(false); } diff --git a/src/session.rs b/src/session.rs index 9be313a..c1cda9b 100644 --- a/src/session.rs +++ b/src/session.rs @@ -16,7 +16,7 @@ use std::{ use crate::{ harness, - protocol::{RecoveryEntry, insert_opt_str, opt_str}, + protocol::{RecoveryEntry, bool_flag, insert_opt_str, opt_str}, task::{pid_is_dead, positive_pid}, }; @@ -34,27 +34,34 @@ pub enum EntryKind { }, } -/// One recipe entry. Serialize a literal without group or name as a bare string. Use an -/// object for every other entry, writing optional fields only when set. +/// One recipe entry. Serialize a literal with no group, no name, and neither mark as a +/// bare string. Use an object for every other entry, writing optional fields only when +/// set and marks only when true. #[derive(Debug, Clone, PartialEq, Eq)] pub struct SessionEntry { pub kind: EntryKind, pub group: Option, pub name: Option, + /// The `m` "in use" tag. + pub tagged: bool, + /// The `]` mark. At most one entry per file: `from_json` refuses a second. + pub flagship: bool, } #[cfg(test)] impl SessionEntry { - /// Unlabelled literal entry. + /// Unlabelled, unmarked literal entry. pub fn literal(cmd: &str) -> Self { Self { kind: EntryKind::Literal(cmd.into()), group: None, name: None, + tagged: false, + flagship: false, } } - /// Unlabelled managed entry. + /// Unlabelled, unmarked managed entry. pub fn managed(agent: &str, resume: Option<&str>) -> Self { Self { kind: EntryKind::Managed { @@ -63,6 +70,8 @@ impl SessionEntry { }, group: None, name: None, + tagged: false, + flagship: false, } } } @@ -123,8 +132,10 @@ fn dirs_json(cfg: &SessionConfig) -> jzon::JsonValue { let mut arr = jzon::JsonValue::new_array(); for e in entries { let mut m = match &e.kind { - // Use the string form for a literal without optional labels. - EntryKind::Literal(cmd) if e.group.is_none() && e.name.is_none() => { + // Use the string form for a literal without optional labels or marks. + EntryKind::Literal(cmd) + if e.group.is_none() && e.name.is_none() && !e.tagged && !e.flagship => + { let _ = arr.push(cmd.as_str()); continue; } @@ -137,6 +148,14 @@ fn dirs_json(cfg: &SessionConfig) -> jzon::JsonValue { }; insert_opt_str(&mut m, "group", e.group.as_deref()); insert_opt_str(&mut m, "name", e.name.as_deref()); + // Omit false marks so an unmarked file is byte-identical to one written + // before marks were stored. + if e.tagged { + let _ = m.insert("tagged", true); + } + if e.flagship { + let _ = m.insert("flagship", true); + } let _ = arr.push(m); } let _ = dirs.insert(dir, arr); @@ -165,10 +184,11 @@ pub fn fingerprint_json(cfg: &SessionConfig) -> String { /// including an array under a directory named `dirs`. Require an integer `version` from 1 /// through [`FORMAT_VERSION`]. Treat a missing version as 1. Accept entry forms regardless /// of version: strings and `{"cmd"}` objects for literals, `{"agent"}` objects for managed -/// agents. Reject the whole file for objects with both keys, unregistered agents, or resume -/// IDs that are not strict UUIDs. Never rewrite an entry: a v1 file loads verbatim, every -/// entry literal. The rewrite that converted v1 agent commands to managed entries (v1 had no -/// managed form) shipped in 0.15.0 and was retired in the release after it. +/// agents. Reject the whole file for objects with both keys, unregistered agents, resume +/// IDs that are not strict UUIDs, non-boolean `tagged` or `flagship` fields, or a second +/// `flagship`. Never rewrite an entry: a v1 file loads verbatim, every entry literal. The +/// rewrite that converted v1 agent commands to managed entries (v1 had no managed form) +/// shipped in 0.15.0 and was retired in the release after it. fn from_json(text: &str) -> io::Result<(Option, SessionConfig)> { let invalid = |message| io::Error::new(io::ErrorKind::InvalidData, message); let parsed = jzon::parse(text).map_err(|e| invalid(e.to_string()))?; @@ -204,6 +224,9 @@ fn from_json(text: &str) -> io::Result<(Option, SessionConfig)> { (None, &parsed, true) }; let mut cfg = SessionConfig::new(); + // Location of the first flagged entry, in file order: the supervisor holds one + // flagship id, so a second entry has no task state to land in. + let mut flagship_at: Option = None; for (dir, val) in dirs.entries() { // In a flat file, `version` is metadata beside the directory keys. if flat && dir == "version" { @@ -219,6 +242,8 @@ fn from_json(text: &str) -> io::Result<(Option, SessionConfig)> { kind: EntryKind::Literal(cmd.to_string()), group: None, name: None, + tagged: false, + flagship: false, }); continue; } @@ -268,11 +293,29 @@ fn from_json(text: &str) -> io::Result<(Option, SessionConfig)> { })?; EntryKind::Literal(cmd.to_string()) }; - entries.push(SessionEntry { + let mark = |field| { + bool_flag(&member[field]).ok_or_else(|| { + invalid(format!( + "{location}, field {field:?}: expected a boolean or null" + )) + }) + }; + let entry = SessionEntry { kind, group: label("group")?, name: label("name")?, - }); + tagged: mark("tagged")?, + flagship: mark("flagship")?, + }; + if entry.flagship { + if let Some(first) = &flagship_at { + return Err(invalid(format!( + "{location}: a second flagship; {first} is already marked" + ))); + } + flagship_at = Some(location); + } + entries.push(entry); } cfg.insert(dir.to_string(), entries); } @@ -734,6 +777,84 @@ mod tests { ); } + /// Round-trip marked entries. A tagged literal leaves the bare-string form; a plain + /// literal keeps it. Marks are written only when true, after the labels. + #[test] + fn round_trips_marked_entries() { + let dir = temp("session_marks_roundtrip"); + let mut cfg = SessionConfig::new(); + cfg.insert( + "~/p".into(), + vec![ + SessionEntry { + tagged: true, + ..e("cargo test") + }, + SessionEntry { + flagship: true, + ..m("claude", Some(ID)) + }, + e("vim"), + ], + ); + + save_in(&dir, "marks", &cfg).unwrap(); + assert_eq!(load_in(&dir, "marks").unwrap(), cfg); + assert_eq!( + fingerprint_json(&cfg), + format!( + r#"{{"~/p":[{{"cmd":"cargo test","tagged":true}},{{"agent":"claude","resume":"{ID}","flagship":true}},"vim"]}}"# + ) + ); + } + + /// `false`, `null`, and omission all leave a mark unset, on both entry forms. + #[test] + fn unset_marks_parse_as_false_in_every_spelling() { + for spelling in [ + "", + r#", "tagged": false, "flagship": false"#, + r#", "tagged": null, "flagship": null"#, + ] { + let text = + format!(r#"{{"d": [{{"cmd": "vim"{spelling}}}, {{"agent": "omp"{spelling}}}]}}"#); + let (_, cfg) = from_json(&text).unwrap(); + assert_eq!(cfg["d"], vec![e("vim"), m("omp", None)], "{text}"); + } + } + + /// Reject the entire file for a non-boolean mark, naming the field, or for a second + /// flagship, naming both locations. A bare-string entry has no field to carry a mark, + /// so it can never be the conflicting entry. + #[test] + fn rejects_malformed_marks() { + let cases = [ + ( + r#"{"d": ["vim", {"cmd": "top", "tagged": "yes"}]}"#, + "directory \"d\", entry 2, field \"tagged\": expected a boolean or null", + ), + ( + r#"{"d": [{"agent": "omp", "flagship": 1}]}"#, + "directory \"d\", entry 1, field \"flagship\": expected a boolean or null", + ), + ( + r#"{"~/a": [{"cmd": "x", "flagship": true}], "~/b": ["y", {"cmd": "z", "flagship": true}]}"#, + "directory \"~/b\", entry 2: a second flagship; \ + directory \"~/a\", entry 1 is already marked", + ), + ( + r#"{"d": [{"agent": "omp", "flagship": true}, {"cmd": "z", "flagship": true}]}"#, + "directory \"d\", entry 2: a second flagship; \ + directory \"d\", entry 1 is already marked", + ), + ]; + for (text, expected) in cases { + let err = from_json(text).unwrap_err(); + assert_eq!(err.kind(), io::ErrorKind::InvalidData, "{text}"); + assert_eq!(err.to_string(), expected, "{text}"); + } + } + /// Include the resume ID in the fingerprint so a cadence pass detects newly captured /// sessions without other fleet changes. #[test] diff --git a/src/supervisor.rs b/src/supervisor.rs index 68a4347..22302d5 100644 --- a/src/supervisor.rs +++ b/src/supervisor.rs @@ -231,13 +231,13 @@ fn affects_recipe(cmd: &Command) -> bool { | Command::Restart { .. } | Command::SetGroup { .. } | Command::SetName { .. } + | Command::Tag { .. } + | Command::Flagship { .. } | Command::LoadSession { .. } | Command::LoadRecovery { .. } => true, - // Preserve the recipe for lifecycle (`Kill`) and dashboard-state - // (`Tag`, `Flagship`) updates, and for the remaining commands. + // `Kill` changes lifecycle, which the recipe does not record. The rest are + // input, view, or meta: they touch nothing `session_config` serializes. Command::Kill { .. } - | Command::Tag { .. } - | Command::Flagship { .. } | Command::Resize { .. } | Command::Watch { .. } | Command::Paste { .. } @@ -559,6 +559,12 @@ impl Supervisor { .is_none_or(|i| self.tasks[i].finished.is_some()) { self.flagship = None; + // The clear is not a command, so `apply` never arms recovery for it. Without + // this, only the 60 s cadence pass would notice, and the snapshot on disk, + // written while the task was live and marked, would stay current for up to a + // minute; a load from it would restore a flagship the core had already + // dropped. + self.recovery.last_mutation = Some(Instant::now()); } let now = Instant::now(); // vte re-checks its ?2026 sync timeout only when bytes arrive, so a @@ -1031,9 +1037,9 @@ impl Supervisor { } } - /// Build `{dir: [entries]}` in spawn order, preserving groups and names. Save managed - /// tasks as agent words with the best-known session IDs; save literals as their exact - /// text. + /// Build `{dir: [entries]}` in spawn order, preserving groups, names, tags, and the + /// flagship mark. Save managed tasks as agent words with the best-known session IDs; + /// save literals as their exact text. fn session_config(&self) -> SessionConfig { // `admit` appends monotonic IDs; `rerun` preserves both index and ID; // removal preserves relative order. @@ -1056,6 +1062,8 @@ impl Supervisor { kind, group: t.group.clone(), name: t.name.clone(), + tagged: t.tagged, + flagship: self.flagship == Some(t.id), }); } cfg @@ -1104,8 +1112,10 @@ impl Supervisor { /// Spawn each recipe entry in its existing directory; skip missing directories. Resolve /// managed binaries as on the Agent page. Record a missing binary as a failed entry - /// with a diagnostic, then continue loading. Return the outcome for one summary line, - /// or `None` after reporting a missing launch context. + /// with a diagnostic, then continue loading. Restore each admitted entry's tag; restore + /// a stored flagship only when no task holds the mark, since load is additive and the + /// live mark outranks the file. Return the outcome for one summary line, or `None` + /// after reporting a missing launch context. fn materialize(&mut self, cfg: &SessionConfig) -> Option { let launch = self.launch_or_refuse()?; let mut out = LoadOutcome::default(); @@ -1147,9 +1157,15 @@ impl Supervisor { entry.group.clone(), entry.name.clone(), ) { - Ok((_, notice)) => { + Ok((id, notice)) => { out.spawned += 1; out.notes.extend(notice); + if entry.tagged { + self.with_task(id, |t| t.tagged = true); + } + if entry.flagship && self.flagship.is_none() { + self.flagship = Some(id); + } } // Track spawn failures separately from skipped entries. Err(e) => { diff --git a/src/supervisor_tests.rs b/src/supervisor_tests.rs index 3a43c0c..ac8726d 100644 --- a/src/supervisor_tests.rs +++ b/src/supervisor_tests.rs @@ -77,6 +77,35 @@ fn session_config_groups_by_dir_in_spawn_order() { assert_eq!(cfg["/tmp"], vec![SessionEntry::literal("b")]); } +/// The recipe carries the `m` tag and the flagship mark of each task. +#[test] +fn session_config_carries_marks() { + let mut s = sup(24, 80); + spawn(&mut s, "sleep 30", here()); + spawn(&mut s, "sleep 31", here()); + let ids: Vec = snapshot(&mut s).iter().map(|t| t.id).collect(); + s.apply(Command::Tag { + id: ids[0], + on: true, + }); + s.apply(Command::Flagship { id: Some(ids[1]) }); + + let cfg = s.session_config(); + assert_eq!( + cfg[&path::abbreviate(&here())], + vec![ + SessionEntry { + tagged: true, + ..SessionEntry::literal("sleep 30") + }, + SessionEntry { + flagship: true, + ..SessionEntry::literal("sleep 31") + }, + ] + ); +} + /// Verify that `tick` queues only a `Tasks` snapshot when no task is watched, /// then queues a `Screen` for the watched task after `Watch`. #[test] @@ -1815,9 +1844,10 @@ fn take_dirty(s: &mut Supervisor) -> bool { s.recovery.last_mutation.take().is_some() } -/// Recipe-changing commands arm recovery even when rejected; tags do not. +/// Recipe-changing commands, marks included, arm recovery even when rejected; `Kill` +/// changes only lifecycle and does not. #[test] -fn recovery_arms_on_structural_mutations_not_tag() { +fn recovery_arms_on_recipe_mutations_not_kill() { let mut s = sup(24, 80); assert!(!take_dirty(&mut s), "a fresh supervisor starts clean"); @@ -1827,9 +1857,15 @@ fn recovery_arms_on_structural_mutations_not_tag() { s.recovery.last_mutation = None; // Reset after the tick in first_id. s.apply(Command::Tag { id, on: true }); + assert!(take_dirty(&mut s), "Tag must arm"); + + s.apply(Command::Flagship { id: Some(id) }); + assert!(take_dirty(&mut s), "Flagship must arm"); + + s.apply(Command::Kill { id }); assert!( !take_dirty(&mut s), - "Tag is not recipe state and must not arm" + "Kill is lifecycle, not recipe state, and must not arm" ); s.apply(Command::SetGroup { @@ -2106,18 +2142,18 @@ fn rerun_clears_flagship() { assert_eq!(s.flagship, None); } -/// Do not arm recovery when marking or clearing the flagship. +/// Arm recovery when marking or clearing the flagship: both change the stored mark. #[test] -fn flagship_does_not_affect_recipe() { +fn flagship_mark_and_clear_arm_recovery() { let mut s = sup(24, 80); spawn(&mut s, "sleep 30", here()); let id = spawned_id(&mut s); s.recovery.last_mutation = None; // Reset after spawning to test the mark in isolation. s.apply(Command::Flagship { id: Some(id) }); - assert!(!take_dirty(&mut s), "marking must not arm"); + assert!(take_dirty(&mut s), "marking must arm"); s.apply(Command::Flagship { id: None }); - assert!(!take_dirty(&mut s), "clearing must not arm"); + assert!(take_dirty(&mut s), "clearing must arm"); } /// Session listings include recovery metadata in descending stem order. @@ -2227,6 +2263,141 @@ fn load_recovery_materializes_the_fleet_and_notices() { assert_eq!(by_cmd(&s, "sleep 31"), (None, Some("web".into()))); } +/// Loading restores each entry's tag and the stored flagship. A flagship already held +/// outranks a stored one: the loaded task is admitted unmarked and the live mark stays. +#[test] +fn load_applies_marks_and_a_live_flagship_wins() { + let dir = scratch("load_marks"); + let config = dir.join("config"); + let mut s = sup_ctx(config_ctx(&config, dir.to_path_buf(), &[])); + let cfg = SessionConfig::from([( + dir.to_string_lossy().into_owned(), + vec![ + SessionEntry { + tagged: true, + ..SessionEntry::literal("sleep 30") + }, + SessionEntry { + flagship: true, + ..SessionEntry::literal("sleep 31") + }, + ], + )]); + session::save_in(&config.join("sessions"), "marks", &cfg).unwrap(); + + s.apply(Command::LoadSession { + name: "marks".into(), + }); + let views = snapshot(&mut s); + let marks: Vec<(&str, bool, bool)> = views + .iter() + .map(|t| (t.command.as_str(), t.tagged, t.flagship)) + .collect(); + assert_eq!( + marks, + [("sleep 30", true, false), ("sleep 31", false, true)] + ); + + // The live flagship is moved to a direct spawn; a second load must not take it back. + spawn(&mut s, "sleep 32", dir.to_path_buf()); + let live = snapshot(&mut s).last().unwrap().id; + s.apply(Command::Flagship { id: Some(live) }); + s.apply(Command::LoadSession { + name: "marks".into(), + }); + let views = snapshot(&mut s); + assert_eq!(views.len(), 5, "load is additive"); + let flagged: Vec = views.iter().filter(|t| t.flagship).map(|t| t.id).collect(); + assert_eq!(flagged, [live], "the live flagship must survive a load"); + assert!( + views.iter().filter(|t| t.id > live).all(|t| !t.flagship), + "a stored flagship must not displace the live one" + ); + assert_eq!( + views.iter().filter(|t| t.tagged).count(), + 2, + "tags are restored on every load" + ); +} + +/// A debounced snapshot follows `Tag`: the file is rewritten with the mark. +#[test] +fn recovery_snapshot_carries_a_tag() { + let dir = scratch("recovery_tag"); + let config = dir.join("config"); + let mut s = recovery_sup( + &config, + dir.to_path_buf(), + Duration::from_millis(100), + Duration::from_secs(600), + ); + let snapshot_text = |config: &Path| { + let files = recovery_files(config); + assert_eq!( + files.len(), + 1, + "one incarnation owns one snapshot: {files:?}" + ); + std::fs::read_to_string(config.join("sessions").join("recovery").join(&files[0])).unwrap() + }; + spawn(&mut s, "sleep 30", dir.to_path_buf()); + let id = first_id(&mut s); + assert!( + wait_until(Duration::from_secs(5), || { + s.tick(); + !recovery_files(&config).is_empty() + }), + "the first snapshot never landed" + ); + assert!( + !snapshot_text(&config).contains("tagged"), + "an unmarked fleet writes no mark field" + ); + + s.apply(Command::Tag { id, on: true }); + assert!( + wait_until(Duration::from_secs(5), || { + s.tick(); + snapshot_text(&config).contains("\"tagged\": true") + }), + "the snapshot was never rewritten with the tag: {}", + snapshot_text(&config) + ); +} + +/// Clearing the flagship on task exit arms a debounced pass; a tick with nothing to +/// clear does not. +#[test] +fn tick_flagship_clear_arms_recovery() { + let mut s = sup(24, 80); + spawn(&mut s, "sleep 30", here()); + let id = first_id(&mut s); + s.apply(Command::Flagship { id: Some(id) }); + assert!( + view_of(&mut s, id).flagship, + "premise: the live task is marked" + ); + assert!(take_dirty(&mut s), "Flagship must arm"); + // `snapshot` drains: a bare tick would leave this view queued ahead of the + // post-exit one `view_of` reads below. + assert!( + snapshot(&mut s)[0].flagship, + "a tick with nothing to clear keeps the mark" + ); + assert!( + !take_dirty(&mut s), + "a tick with nothing to clear must not arm" + ); + + s.apply(Command::Kill { id }); + wait_exited(&mut s, id); + assert!(!take_dirty(&mut s), "neither Kill nor reap arms by itself"); + assert!(!view_of(&mut s, id).flagship, "the mark is cleared on exit"); + assert!(take_dirty(&mut s), "the clear must arm"); + s.tick(); + assert!(!take_dirty(&mut s), "arm only on the tick that clears"); +} + /// Unknown and path-shaped recovery stems fail without spawning tasks. #[test] fn load_recovery_refuses_unknown_and_traversal_stems() { From cbc2911b03224e274f4dc85a1c961aa54879d804 Mon Sep 17 00:00:00 2001 From: Christopher Sardegna Date: Sun, 18 Oct 2026 16:19:22 -0700 Subject: [PATCH 2/3] Refactor flagship handling in Supervisor to ensure proper clearing on task exit and enhance recovery tests for detached maintenance scenarios. --- src/supervisor.rs | 54 +++++++++------- src/supervisor_tests.rs | 134 +++++++++++++++++++++++++++++++--------- 2 files changed, 138 insertions(+), 50 deletions(-) diff --git a/src/supervisor.rs b/src/supervisor.rs index 22302d5..551e14f 100644 --- a/src/supervisor.rs +++ b/src/supervisor.rs @@ -308,9 +308,10 @@ pub struct Supervisor { watched: Option, /// Whether the current watch permits clipboard forwarding. watch_attached: bool, - /// The flagship task id, or `None`: at most one id is representable. - /// Clear in `tick` if the task is finished or absent, before including - /// the mark in a snapshot. + /// The flagship task id, or `None`: at most one id is representable. `reap` clears + /// it once the task is finished or absent. Between reaps it can name a task that + /// `Remove` dropped or whose exit is not yet latched, so decisions that need a live + /// holder read it through `flagship_holder`. flagship: Option, /// The last emitted screen fingerprint. `lines` stays empty because only /// emitted copies carry them. Cleared when `watched` changes to force a @@ -525,6 +526,30 @@ impl Supervisor { } } self.graveyard.retain_mut(|t| !t.try_collect()); + // Check current state rather than individual transitions to handle exit, + // removal, session load, and marks placed on already-finished tasks. The check + // lives here, not in `tick`, because the detached daemon reaps without ticking: + // a flagship that exited between clients would otherwise keep its mark in every + // snapshot until the next connection. + if self.flagship.is_some() && self.flagship_holder().is_none() { + self.flagship = None; + // The clear is not a command, so `apply` never arms recovery for it. Without + // this, only the 60 s cadence pass would notice, and the snapshot on disk, + // written while the task was live and marked, would stay current for up to a + // minute; a load from it would restore a flagship the core had already + // dropped. + self.recovery.last_mutation = Some(Instant::now()); + } + } + + /// The flagship's id while an unfinished task in the set holds it. `flagship` itself + /// may be stale between reaps: `Remove` leaves it set, and an exit is latched only by + /// `reap`. + fn flagship_holder(&self) -> Option { + self.flagship.filter(|&id| { + self.index_of(id) + .is_some_and(|i| self.tasks[i].finished.is_none()) + }) } /// TERM every owned group, wait one shared grace, then SIGKILL before @@ -551,21 +576,6 @@ impl Supervisor { /// client through `drain`ed events, never through a `Task`. pub fn tick(&mut self) { self.reap(); - // Check current state rather than individual transitions to handle exit, - // removal, session load, and marks placed on already-finished tasks. - if let Some(id) = self.flagship - && self - .index_of(id) - .is_none_or(|i| self.tasks[i].finished.is_some()) - { - self.flagship = None; - // The clear is not a command, so `apply` never arms recovery for it. Without - // this, only the 60 s cadence pass would notice, and the snapshot on disk, - // written while the task was live and marked, would stay current for up to a - // minute; a load from it would restore a flagship the core had already - // dropped. - self.recovery.last_mutation = Some(Instant::now()); - } let now = Instant::now(); // vte re-checks its ?2026 sync timeout only when bytes arrive, so a // child that opens BSU and stalls would freeze its view. This tick is @@ -1113,8 +1123,8 @@ impl Supervisor { /// Spawn each recipe entry in its existing directory; skip missing directories. Resolve /// managed binaries as on the Agent page. Record a missing binary as a failed entry /// with a diagnostic, then continue loading. Restore each admitted entry's tag; restore - /// a stored flagship only when no task holds the mark, since load is additive and the - /// live mark outranks the file. Return the outcome for one summary line, or `None` + /// a stored flagship only when no live task holds the mark, since load is additive and + /// the live mark outranks the file. Return the outcome for one summary line, or `None` /// after reporting a missing launch context. fn materialize(&mut self, cfg: &SessionConfig) -> Option { let launch = self.launch_or_refuse()?; @@ -1163,7 +1173,9 @@ impl Supervisor { if entry.tagged { self.with_task(id, |t| t.tagged = true); } - if entry.flagship && self.flagship.is_none() { + // `flagship` may still name a task `Remove` dropped since the last + // reap; only a live holder outranks the stored mark. + if entry.flagship && self.flagship_holder().is_none() { self.flagship = Some(id); } } diff --git a/src/supervisor_tests.rs b/src/supervisor_tests.rs index ac8726d..d797e11 100644 --- a/src/supervisor_tests.rs +++ b/src/supervisor_tests.rs @@ -1839,6 +1839,17 @@ fn recovery_files(config: &Path) -> Vec { names } +/// The one recovery snapshot under `config`'s session root, as text. +fn recovery_text(config: &Path) -> String { + let files = recovery_files(config); + assert_eq!( + files.len(), + 1, + "one incarnation owns one snapshot: {files:?}" + ); + std::fs::read_to_string(config.join("sessions").join("recovery").join(&files[0])).unwrap() +} + /// Check for a pending debounced recipe pass, then clear its timestamp. fn take_dirty(s: &mut Supervisor) -> bool { s.recovery.last_mutation.take().is_some() @@ -2331,15 +2342,6 @@ fn recovery_snapshot_carries_a_tag() { Duration::from_millis(100), Duration::from_secs(600), ); - let snapshot_text = |config: &Path| { - let files = recovery_files(config); - assert_eq!( - files.len(), - 1, - "one incarnation owns one snapshot: {files:?}" - ); - std::fs::read_to_string(config.join("sessions").join("recovery").join(&files[0])).unwrap() - }; spawn(&mut s, "sleep 30", dir.to_path_buf()); let id = first_id(&mut s); assert!( @@ -2350,7 +2352,7 @@ fn recovery_snapshot_carries_a_tag() { "the first snapshot never landed" ); assert!( - !snapshot_text(&config).contains("tagged"), + !recovery_text(&config).contains("tagged"), "an unmarked fleet writes no mark field" ); @@ -2358,44 +2360,118 @@ fn recovery_snapshot_carries_a_tag() { assert!( wait_until(Duration::from_secs(5), || { s.tick(); - snapshot_text(&config).contains("\"tagged\": true") + recovery_text(&config).contains("\"tagged\": true") }), "the snapshot was never rewritten with the tag: {}", - snapshot_text(&config) + recovery_text(&config) ); } -/// Clearing the flagship on task exit arms a debounced pass; a tick with nothing to -/// clear does not. +/// The reap that latches the flagship's exit clears the mark and arms a debounced pass; +/// a reap with nothing to clear does neither. `reap`, not `tick`, owns the clear because +/// the detached daemon reaps without ticking. #[test] -fn tick_flagship_clear_arms_recovery() { +fn reap_clears_an_exited_flagship_and_arms_recovery() { let mut s = sup(24, 80); spawn(&mut s, "sleep 30", here()); let id = first_id(&mut s); s.apply(Command::Flagship { id: Some(id) }); - assert!( - view_of(&mut s, id).flagship, - "premise: the live task is marked" - ); assert!(take_dirty(&mut s), "Flagship must arm"); - // `snapshot` drains: a bare tick would leave this view queued ahead of the - // post-exit one `view_of` reads below. - assert!( - snapshot(&mut s)[0].flagship, - "a tick with nothing to clear keeps the mark" + s.reap(); + assert_eq!( + s.flagship, + Some(id), + "a reap with nothing to clear keeps the mark" ); assert!( !take_dirty(&mut s), - "a tick with nothing to clear must not arm" + "a reap with nothing to clear must not arm" ); s.apply(Command::Kill { id }); + assert!(!take_dirty(&mut s), "Kill does not arm"); wait_exited(&mut s, id); - assert!(!take_dirty(&mut s), "neither Kill nor reap arms by itself"); - assert!(!view_of(&mut s, id).flagship, "the mark is cleared on exit"); + assert_eq!( + s.flagship, None, + "the reap that latches the exit clears the mark" + ); assert!(take_dirty(&mut s), "the clear must arm"); - s.tick(); - assert!(!take_dirty(&mut s), "arm only on the tick that clears"); + s.reap(); + assert!(!take_dirty(&mut s), "arm only on the reap that clears"); +} + +/// Between clients the daemon reaps and runs maintenance without ticking. An exited +/// flagship must leave the snapshot on that path too, or the next load restores it. +#[test] +fn detached_maintenance_drops_an_exited_flagship_from_the_snapshot() { + let dir = scratch("detached_mark"); + let config = dir.join("config"); + let mut s = recovery_sup( + &config, + dir.to_path_buf(), + Duration::from_millis(100), + Duration::from_secs(600), + ); + spawn(&mut s, "sleep 30", dir.to_path_buf()); + let id = first_id(&mut s); + s.apply(Command::Flagship { id: Some(id) }); + assert!( + wait_until(Duration::from_secs(5), || { + s.recovery_maintenance(); + recovery_files(&config).len() == 1 + && recovery_text(&config).contains("\"flagship\": true") + }), + "the marked snapshot never landed" + ); + + s.apply(Command::Kill { id }); + wait_exited(&mut s, id); + assert!( + wait_until(Duration::from_secs(5), || { + s.recovery_maintenance(); + !recovery_text(&config).contains("\"flagship\": true") + }), + "detached maintenance kept the exited flagship: {}", + recovery_text(&config) + ); +} + +/// `Remove` leaves the flagship id behind until a reap. Loaded before that reap, a +/// stored mark must still be applied: nothing live holds the flagship, so the stale id +/// has no precedence. +#[test] +fn load_flagship_is_not_blocked_by_a_removed_holder() { + let dir = scratch("load_stale_flagship"); + let config = dir.join("config"); + let mut s = sup_ctx(config_ctx(&config, dir.to_path_buf(), &[])); + let cfg = SessionConfig::from([( + dir.to_string_lossy().into_owned(), + vec![SessionEntry { + flagship: true, + ..SessionEntry::literal("sleep 31") + }], + )]); + session::save_in(&config.join("sessions"), "flag", &cfg).unwrap(); + + spawn(&mut s, "sleep 30", dir.to_path_buf()); + let id = first_id(&mut s); + s.apply(Command::Flagship { id: Some(id) }); + s.apply(Command::Remove { id }); + assert_eq!( + s.flagship, + Some(id), + "premise: Remove leaves the id behind until a reap" + ); + + s.apply(Command::LoadSession { + name: "flag".into(), + }); + let views = snapshot(&mut s); + assert_eq!(views.len(), 1, "the removed task is gone"); + assert!( + views[0].flagship, + "a stale holder must not suppress the stored mark" + ); } /// Unknown and path-shaped recovery stems fail without spawning tasks. From 8cabf7d5497e370f82cb5dd093ff6985eb744be8 Mon Sep 17 00:00:00 2001 From: Christopher Sardegna Date: Sun, 18 Oct 2026 16:30:41 -0700 Subject: [PATCH 3/3] Update documentation and comments for session marks and flagship handling --- docs/README.md | 2 +- docs/commands.md | 2 +- docs/sessions.md | 22 +++++++++++++------- src/protocol.rs | 2 +- src/session.rs | 25 +++++++++++----------- src/supervisor.rs | 46 +++++++++++++++++++---------------------- src/supervisor_tests.rs | 33 ++++++++++++++--------------- 7 files changed, 67 insertions(+), 65 deletions(-) diff --git a/docs/README.md b/docs/README.md index bbca19a..fdc5180 100644 --- a/docs/README.md +++ b/docs/README.md @@ -165,7 +165,7 @@ The runtime directory is validated before its contents are used. The path must b Full command lines are persisted in recipes, including any embedded secrets. A token passed as an argument is written to its session file and to every recovery snapshot that captures the task. -The client environment is not persisted. Each client's environment and working directory are sent during the connection handshake and retained in daemon memory. Only directories, commands, group assignments, display names, tags, and the flagship are stored in session and recovery files. +The client environment is not persisted. Each client's environment and working directory are sent during the connection handshake and retained in daemon memory. Use session and recovery files to preserve only directories, commands, group assignments, display names, tags, and the flagship. ### Captured IDs diff --git a/docs/commands.md b/docs/commands.md index efc192e..b1ba61e 100644 --- a/docs/commands.md +++ b/docs/commands.md @@ -155,7 +155,7 @@ Press `Ctrl-]` from the dashboard, peek, or an attached task to attach to the fl After attaching to the flagship with `Enter`, press `Ctrl-]` to return to the dashboard. -The flagship mark is cleared on task completion or removal. Tags set with `m` are retained after task completion. Both marks are saved in [sessions](sessions.md) and recovery snapshots and restored on load; a stored flagship is applied only when no task is currently marked. +You keep tags set with `m` after task completion, but lose the flagship mark on completion or removal. Save and load [sessions](sessions.md) to preserve and restore both marks. You can also recover both marks from automatic snapshots. When loading with a current flagship, you keep that mark. #### Renaming diff --git a/docs/sessions.md b/docs/sessions.md index f7ae736..b27ada6 100644 --- a/docs/sessions.md +++ b/docs/sessions.md @@ -1,6 +1,6 @@ # Sessions -A session is the saved state of the task set as the daemon holds it. Stored: working directories, commands or managed agents with their session IDs, group assignments, display names, "in use" tags, and the flagship mark. Not stored: anything the client owns (the attached or peeked task, the selection, grouping and sort modes, terminal size) and anything derived from the process (lifecycle, output, timings). Loading a session always starts new processes. To keep live processes running across client disconnects, use the [daemon](README.md#storage-paths). +Save a session to preserve the task set's working directories, commands or managed agents with their session IDs, group assignments, display names, "in use" tags, and flagship mark. You cannot preserve client state (the attached or peeked task, selection, grouping and sort modes, terminal size) or process state (lifecycle, output, timings) in a session. Each time you load a session, you start new processes with the saved settings. To keep live processes running across client disconnects, use the [daemon](README.md#storage-paths). ## Storage @@ -20,10 +20,10 @@ The filename is derived from the session name: leading and trailing whitespace i A session file is a JSON object with three fields: `version` for the format version, currently 2; `name` for the typed session name, trimmed but not sanitized; and `dirs` for an ordered list of entries per working directory. Use one of two entry forms, according to the task's launch mode: -- A **literal** entry is a typed command. Use a bare string without labels or marks, or an object with `cmd` and optional `group`, `name`, `tagged`, and `flagship` fields. On load, the stored text is run unchanged through your shell: even `"claude"` is a literal command. -- A **managed** entry is an object with `agent`, the registered program word (`claude`, `codex`, `grok`, or `omp`), plus optional `resume`, `group`, `name`, `tagged`, and `flagship` fields. Tasks launched from the Agent page are saved in this form. On load, the agent is launched as managed; with `resume`, the stored session ID is reopened, and without it, a fresh conversation is started. See [Agent session resume](agent-resume.md). +- A **literal** entry is a typed command. Use a bare string without labels or marks, or an object with `cmd` and optional `group`, `name`, `tagged`, and `flagship` fields. Load a literal entry to run the stored text unchanged through your shell: even `"claude"` is a literal command. +- A **managed** entry is an object with `agent`, the registered program word (`claude`, `codex`, `grok`, or `omp`), plus optional `resume`, `group`, `name`, `tagged`, and `flagship` fields. Save tasks launched from the Agent page in this form. Load a managed entry with `resume` to reopen the stored session ID, or without it to start a fresh conversation. See [Agent session resume](agent-resume.md). -`tagged` and `flagship` are optional booleans on both object forms: with `true`, the task is tagged ["in use"](commands.md#grouping-and-tagging) or made the [flagship](commands.md#the-flagship); with `false`, `null`, or omission, it is unmarked. Each is written only when true. At most one entry per file may carry `flagship`; with a second, the whole load is refused. +Use the optional booleans `tagged` and `flagship` on either object form. Set `tagged` to `true` to tag a task ["in use"](commands.md#grouping-and-tagging), or `flagship` to `true` to mark the [flagship](commands.md#the-flagship). To leave either mark unset, use `false`, `null`, or omit the field. On save, include each field only when true. Mark at most one entry per file as the flagship; with a second, you cannot load the file. ```json { @@ -56,7 +56,15 @@ Saves are atomic: a private temporary file is written and synced in the session The file is plain JSON and practical to edit by hand. Edit `name` to change the stored session identity: a subsequent save under the old name will be refused by the collision check. A readable stored name is still used for collision checks and picker labels when the command body is invalid or the version unsupported. -The entire recipe is validated before any commands are started. The root must be an object, every directory value must be an array, and every entry must be a command string or an object with a string `cmd` or a string `agent`, never both. `agent` must be a supported program word, and `resume`, when present, must be a lowercase `8-4-4-4-12` UUID or `null`. Strings, `null`, or omission are accepted for optional entry `group` and `name` fields and the wrapped session's `name`; booleans, `null`, or omission for `tagged` and `flagship`; other types are rejected. A second `flagship` is rejected, with both entry locations in the error. Unknown fields in wrapped metadata and entry objects are ignored. Empty maps, arrays, and strings are valid. With invalid JSON or any malformed field, the whole load is refused; existing tasks and the recipe file are left intact. In schema errors, the quoted directory key, entry number (starting at 1), and offending field are included where applicable. +On load, expect validation of the entire recipe before any commands are started. Use this schema: + +- Use an object for the root and an array for each directory value. +- For each entry, use a command string or an object with a string `cmd` or a string `agent`, never both. +- For `agent`, use a supported program word. For `resume`, use a lowercase `8-4-4-4-12` UUID, `null`, or omit the field. +- For the optional entry `group` and `name` fields and the wrapped session's `name`, use strings, `null`, or omit the fields. +- For `tagged` and `flagship`, use booleans, `null`, or omit the fields. Set `flagship` to `true` on at most one entry; otherwise, both entry locations are reported in the error. + +You can include unknown fields in wrapped metadata and entry objects; these are ignored. You can also use empty maps, arrays, and strings. With invalid JSON or any malformed field, you cannot load any part of the recipe; your existing tasks and recipe file remain intact. For schema errors, expect the quoted directory key, entry number (starting at 1), and offending field where applicable. After validation, group and display names are stripped of control characters and surrounding whitespace and limited to 64 characters. `Unassigned` is treated as no group but is a legal display name. Missing directories, entries past the task limit, and literal commands past the length limit are skipped and counted in the load status. A managed entry whose agent is not found on the connected client's `PATH` is reported on the status line and counted as a failed spawn; the remaining entries are still loaded. Spawn failures are reported separately; tasks already started from a structurally valid recipe are left running. @@ -66,7 +74,7 @@ Use the string form for commands with no group, no name, and no mark. String and The current task set is automatically snapshotted under `recovery/` inside the session directory. A separate file is written for each daemon (or `--foreground` core), named for its start time and process ID. After each write, the new file and files whose process IDs are still live are protected; among the remaining files, the nine newest names are retained. A shared recovery directory can therefore contain more than ten snapshots while multiple writers are live. -A snapshot pass is scheduled two seconds after the last command that can change a saved recipe, coalescing a burst of commands. Marking with `m` or `]` is such a command. The flagship clear on task exit is not a command, but a pass is scheduled for it too. A nonempty recipe is written when its content or destination changed, or when the expected snapshot file is missing. Content changes that no command caused, such as a managed task's newly captured session ID, are also checked every 60 seconds. +Expect a snapshot pass two seconds after the last command that can change a saved recipe, including marking with `m` or `]`. After a burst of commands, count the delay from the last command. You also get a pass after the flagship mark is cleared on task exit, without a command. For a nonempty recipe, expect a write when its content or destination changed, or when the expected snapshot file is missing. For changes made without a command, such as a newly captured session ID for a managed task, expect checks every 60 seconds. - No snapshot is written for an empty fleet. After removing every task, the previous snapshot is retained. - Snapshots are left in place after quitting, disconnecting, or `fleetcom --kill`. @@ -83,4 +91,4 @@ As with saved recipes, full command lines are persisted in recovery files, inclu - Load in-app: `o`, pick from the list, `Enter`. - Load at launch: `fleetcom `. -On load, new processes are always spawned from the stored commands. Live process state is retained by the daemon and never stored in session files. Load is additive and never disturbs live marks: tags are restored on every loaded task, but a stored flagship is applied only when no task is currently marked. See [Agent session resume](agent-resume.md) for preserving supported agent conversations across relaunch. +Load a session to start new processes from the stored commands. To retain live process state, use the daemon; you cannot store that state in session files. When you load, you keep your existing tasks and marks and recover the tags on every loaded task. You recover a stored flagship only when no task is currently marked. See [Agent session resume](agent-resume.md) for preserving supported agent conversations across relaunch. diff --git a/src/protocol.rs b/src/protocol.rs index 4576ac8..8363d76 100644 --- a/src/protocol.rs +++ b/src/protocol.rs @@ -453,7 +453,7 @@ fn num_from>(v: &jzon::JsonValue) -> Option { } /// Decode an optional boolean flag. Use `false` for missing or null values; reject the -/// message for other non-boolean values. Session entry marks use the same rule. +/// message for other non-boolean values. Apply the same rule to session entry marks. pub fn bool_flag(v: &jzon::JsonValue) -> Option { if v.is_null() { return Some(false); diff --git a/src/session.rs b/src/session.rs index c1cda9b..6b8e027 100644 --- a/src/session.rs +++ b/src/session.rs @@ -44,7 +44,7 @@ pub struct SessionEntry { pub name: Option, /// The `m` "in use" tag. pub tagged: bool, - /// The `]` mark. At most one entry per file: `from_json` refuses a second. + /// The `]` mark. Allow at most one flagged entry per file; reject a second in `from_json`. pub flagship: bool, } @@ -148,8 +148,7 @@ fn dirs_json(cfg: &SessionConfig) -> jzon::JsonValue { }; insert_opt_str(&mut m, "group", e.group.as_deref()); insert_opt_str(&mut m, "name", e.name.as_deref()); - // Omit false marks so an unmarked file is byte-identical to one written - // before marks were stored. + // Omit false marks to preserve the exact serialization of unmarked entries. if e.tagged { let _ = m.insert("tagged", true); } @@ -186,9 +185,9 @@ pub fn fingerprint_json(cfg: &SessionConfig) -> String { /// of version: strings and `{"cmd"}` objects for literals, `{"agent"}` objects for managed /// agents. Reject the whole file for objects with both keys, unregistered agents, resume /// IDs that are not strict UUIDs, non-boolean `tagged` or `flagship` fields, or a second -/// `flagship`. Never rewrite an entry: a v1 file loads verbatim, every entry literal. The -/// rewrite that converted v1 agent commands to managed entries (v1 had no managed form) -/// shipped in 0.15.0 and was retired in the release after it. +/// `flagship`. Never rewrite an entry: load a v1 file verbatim, with every entry literal. +/// Conversion of v1 agent commands to managed entries (v1 had no managed form) was +/// introduced in 0.15.0 and retired in the release after it. fn from_json(text: &str) -> io::Result<(Option, SessionConfig)> { let invalid = |message| io::Error::new(io::ErrorKind::InvalidData, message); let parsed = jzon::parse(text).map_err(|e| invalid(e.to_string()))?; @@ -224,8 +223,8 @@ fn from_json(text: &str) -> io::Result<(Option, SessionConfig)> { (None, &parsed, true) }; let mut cfg = SessionConfig::new(); - // Location of the first flagged entry, in file order: the supervisor holds one - // flagship id, so a second entry has no task state to land in. + // Retain the first flagged entry's location for diagnostics: only one flagship ID + // can be represented in supervisor state. let mut flagship_at: Option = None; for (dir, val) in dirs.entries() { // In a flat file, `version` is metadata beside the directory keys. @@ -777,8 +776,8 @@ mod tests { ); } - /// Round-trip marked entries. A tagged literal leaves the bare-string form; a plain - /// literal keeps it. Marks are written only when true, after the labels. + /// Round-trip marked entries. Serialize a tagged literal as an object and a plain + /// literal as a bare string. Write marks only when true, after the labels. #[test] fn round_trips_marked_entries() { let dir = temp("session_marks_roundtrip"); @@ -808,7 +807,7 @@ mod tests { ); } - /// `false`, `null`, and omission all leave a mark unset, on both entry forms. + /// Treat `false`, `null`, and omission as an unset mark on both entry forms. #[test] fn unset_marks_parse_as_false_in_every_spelling() { for spelling in [ @@ -824,8 +823,8 @@ mod tests { } /// Reject the entire file for a non-boolean mark, naming the field, or for a second - /// flagship, naming both locations. A bare-string entry has no field to carry a mark, - /// so it can never be the conflicting entry. + /// flagship, naming both locations. Use object fields to store marks; a bare-string + /// entry cannot be the conflicting entry. #[test] fn rejects_malformed_marks() { let cases = [ diff --git a/src/supervisor.rs b/src/supervisor.rs index 551e14f..68d1fdc 100644 --- a/src/supervisor.rs +++ b/src/supervisor.rs @@ -235,8 +235,8 @@ fn affects_recipe(cmd: &Command) -> bool { | Command::Flagship { .. } | Command::LoadSession { .. } | Command::LoadRecovery { .. } => true, - // `Kill` changes lifecycle, which the recipe does not record. The rest are - // input, view, or meta: they touch nothing `session_config` serializes. + // Exclude `Kill`: lifecycle is not part of the recipe. For the remaining input, + // view, and meta commands, no fields serialized by `session_config` are changed. Command::Kill { .. } | Command::Resize { .. } | Command::Watch { .. } @@ -308,10 +308,9 @@ pub struct Supervisor { watched: Option, /// Whether the current watch permits clipboard forwarding. watch_attached: bool, - /// The flagship task id, or `None`: at most one id is representable. `reap` clears - /// it once the task is finished or absent. Between reaps it can name a task that - /// `Remove` dropped or whose exit is not yet latched, so decisions that need a live - /// holder read it through `flagship_holder`. + /// The flagship task ID, or `None`. Clear it in `reap` once the task is finished or + /// absent. Between reaps, the ID may refer to a removed task or one whose exit is not + /// yet latched. Use `flagship_holder` to check for an unfinished task still in the set. flagship: Option, /// The last emitted screen fingerprint. `lines` stays empty because only /// emitted copies carry them. Cleared when `watched` changes to force a @@ -526,25 +525,22 @@ impl Supervisor { } } self.graveyard.retain_mut(|t| !t.try_collect()); - // Check current state rather than individual transitions to handle exit, - // removal, session load, and marks placed on already-finished tasks. The check - // lives here, not in `tick`, because the detached daemon reaps without ticking: - // a flagship that exited between clients would otherwise keep its mark in every - // snapshot until the next connection. + // Check current state to cover exit, removal, session load, and marks placed on + // already-finished tasks. Check during reaping because `tick` is not called between + // clients; otherwise, an exited flagship would remain marked in recovery + // snapshots until the next connection. if self.flagship.is_some() && self.flagship_holder().is_none() { self.flagship = None; - // The clear is not a command, so `apply` never arms recovery for it. Without - // this, only the 60 s cadence pass would notice, and the snapshot on disk, - // written while the task was live and marked, would stay current for up to a - // minute; a load from it would restore a flagship the core had already - // dropped. + // Schedule recovery explicitly after clearing the mark outside `apply`. + // Otherwise, a user can restore the old mark from disk before the 60 s + // cadence pass. self.recovery.last_mutation = Some(Instant::now()); } } - /// The flagship's id while an unfinished task in the set holds it. `flagship` itself - /// may be stale between reaps: `Remove` leaves it set, and an exit is latched only by - /// `reap`. + /// Return the flagship ID only for an unfinished task still in the set. Between + /// reaps, the stored ID may refer to a removed task or one whose exit is not yet + /// latched. fn flagship_holder(&self) -> Option { self.flagship.filter(|&id| { self.index_of(id) @@ -1122,10 +1118,10 @@ impl Supervisor { /// Spawn each recipe entry in its existing directory; skip missing directories. Resolve /// managed binaries as on the Agent page. Record a missing binary as a failed entry - /// with a diagnostic, then continue loading. Restore each admitted entry's tag; restore - /// a stored flagship only when no live task holds the mark, since load is additive and - /// the live mark outranks the file. Return the outcome for one summary line, or `None` - /// after reporting a missing launch context. + /// with a diagnostic, then continue loading. Restore each admitted entry's tag. + /// Preserve the current tasks and their marks: restore a stored flagship only when + /// no live task is marked. Return the outcome for one summary line, or `None` after + /// reporting a missing launch context. fn materialize(&mut self, cfg: &SessionConfig) -> Option { let launch = self.launch_or_refuse()?; let mut out = LoadOutcome::default(); @@ -1173,8 +1169,8 @@ impl Supervisor { if entry.tagged { self.with_task(id, |t| t.tagged = true); } - // `flagship` may still name a task `Remove` dropped since the last - // reap; only a live holder outranks the stored mark. + // Ignore a removed flagship's ID before the next reap. Preserve + // the current mark only for a live holder. if entry.flagship && self.flagship_holder().is_none() { self.flagship = Some(id); } diff --git a/src/supervisor_tests.rs b/src/supervisor_tests.rs index d797e11..dbecd2b 100644 --- a/src/supervisor_tests.rs +++ b/src/supervisor_tests.rs @@ -77,7 +77,7 @@ fn session_config_groups_by_dir_in_spawn_order() { assert_eq!(cfg["/tmp"], vec![SessionEntry::literal("b")]); } -/// The recipe carries the `m` tag and the flagship mark of each task. +/// Include each task's `m` tag and flagship mark in the recipe. #[test] fn session_config_carries_marks() { let mut s = sup(24, 80); @@ -1839,7 +1839,7 @@ fn recovery_files(config: &Path) -> Vec { names } -/// The one recovery snapshot under `config`'s session root, as text. +/// Read the sole recovery snapshot under `config`'s session root as text. fn recovery_text(config: &Path) -> String { let files = recovery_files(config); assert_eq!( @@ -1855,8 +1855,8 @@ fn take_dirty(s: &mut Supervisor) -> bool { s.recovery.last_mutation.take().is_some() } -/// Recipe-changing commands, marks included, arm recovery even when rejected; `Kill` -/// changes only lifecycle and does not. +/// Schedule recovery for recipe-changing commands, including marks and rejected +/// commands. Exclude `Kill`: lifecycle is not part of the recipe. #[test] fn recovery_arms_on_recipe_mutations_not_kill() { let mut s = sup(24, 80); @@ -2153,7 +2153,7 @@ fn rerun_clears_flagship() { assert_eq!(s.flagship, None); } -/// Arm recovery when marking or clearing the flagship: both change the stored mark. +/// Schedule recovery after setting or clearing the flagship mark. #[test] fn flagship_mark_and_clear_arm_recovery() { let mut s = sup(24, 80); @@ -2274,8 +2274,8 @@ fn load_recovery_materializes_the_fleet_and_notices() { assert_eq!(by_cmd(&s, "sleep 31"), (None, Some("web".into()))); } -/// Loading restores each entry's tag and the stored flagship. A flagship already held -/// outranks a stored one: the loaded task is admitted unmarked and the live mark stays. +/// Restore tags and the stored flagship on load. If a live task is already marked, +/// preserve that mark and admit the stored flagship as an unmarked task. #[test] fn load_applies_marks_and_a_live_flagship_wins() { let dir = scratch("load_marks"); @@ -2309,7 +2309,7 @@ fn load_applies_marks_and_a_live_flagship_wins() { [("sleep 30", true, false), ("sleep 31", false, true)] ); - // The live flagship is moved to a direct spawn; a second load must not take it back. + // Mark a directly spawned task, then load again to check that its mark is preserved. spawn(&mut s, "sleep 32", dir.to_path_buf()); let live = snapshot(&mut s).last().unwrap().id; s.apply(Command::Flagship { id: Some(live) }); @@ -2331,7 +2331,7 @@ fn load_applies_marks_and_a_live_flagship_wins() { ); } -/// A debounced snapshot follows `Tag`: the file is rewritten with the mark. +/// Rewrite the recovery snapshot with the tag after the debounce interval. #[test] fn recovery_snapshot_carries_a_tag() { let dir = scratch("recovery_tag"); @@ -2367,9 +2367,9 @@ fn recovery_snapshot_carries_a_tag() { ); } -/// The reap that latches the flagship's exit clears the mark and arms a debounced pass; -/// a reap with nothing to clear does neither. `reap`, not `tick`, owns the clear because -/// the detached daemon reaps without ticking. +/// Clear the mark and schedule recovery when latching the flagship's exit in `reap`. +/// Preserve the recovery schedule when nothing is cleared. Reap directly, without +/// ticking, to cover the detached daemon path. #[test] fn reap_clears_an_exited_flagship_and_arms_recovery() { let mut s = sup(24, 80); @@ -2400,8 +2400,8 @@ fn reap_clears_an_exited_flagship_and_arms_recovery() { assert!(!take_dirty(&mut s), "arm only on the reap that clears"); } -/// Between clients the daemon reaps and runs maintenance without ticking. An exited -/// flagship must leave the snapshot on that path too, or the next load restores it. +/// Reap and run maintenance without ticking to match the detached daemon loop. Remove +/// the exited flagship's mark from the snapshot to avoid restoring it on the next load. #[test] fn detached_maintenance_drops_an_exited_flagship_from_the_snapshot() { let dir = scratch("detached_mark"); @@ -2436,9 +2436,8 @@ fn detached_maintenance_drops_an_exited_flagship_from_the_snapshot() { ); } -/// `Remove` leaves the flagship id behind until a reap. Loaded before that reap, a -/// stored mark must still be applied: nothing live holds the flagship, so the stale id -/// has no precedence. +/// Restore a stored flagship when loading after removal and before reaping. The old ID +/// is still set, but no live task is marked. #[test] fn load_flagship_is_not_blocked_by_a_removed_holder() { let dir = scratch("load_stale_flagship");