diff --git a/completions/pty.bash b/completions/pty.bash index 537a698..26abf02 100644 --- a/completions/pty.bash +++ b/completions/pty.bash @@ -95,7 +95,7 @@ _pty() { fi ;; gc) - COMPREPLY=($(compgen -W "-n --dry-run --idle-days --fast-fail-window --fast-fail-limit --print-launchd-plist --interval" -- "${cur}")) + COMPREPLY=($(compgen -W "-n --dry-run --idle-days --keep-max-age --fast-fail-window --fast-fail-limit --print-launchd-plist --interval" -- "${cur}")) ;; tag) if [[ "${cur}" == -* ]]; then diff --git a/completions/pty.fish b/completions/pty.fish index a44414c..daf0979 100644 --- a/completions/pty.fish +++ b/completions/pty.fish @@ -133,6 +133,7 @@ complete -c pty -n '__pty_using_command recover' -l snapshot -d 'Captured capabi complete -c pty -n '__pty_using_command rm remove' -a '(__pty_sessions)' -d 'Session' complete -c pty -n '__pty_using_command gc' -l dry-run -s n -d 'Preview without changing anything' complete -c pty -n '__pty_using_command gc' -l idle-days -d 'Reap permanents with no attach in N days' +complete -c pty -n '__pty_using_command gc' -l keep-max-age -d 'Keep-tag retention for dead sessions (default 7d; 0 = now)' complete -c pty -n '__pty_using_command gc' -l fast-fail-window -d 'Fast-fail window (seconds; default 60)' complete -c pty -n '__pty_using_command gc' -l fast-fail-limit -d 'Consecutive fast fails before flapping (default 3)' complete -c pty -n '__pty_using_command gc' -l print-launchd-plist -d 'Emit a launchd plist that runs pty gc' diff --git a/completions/pty.zsh b/completions/pty.zsh index 9b014e9..af8df55 100644 --- a/completions/pty.zsh +++ b/completions/pty.zsh @@ -149,6 +149,7 @@ _pty() { _arguments \ '(n --dry-run){n,--dry-run}[Preview without changing anything]' \ '--idle-days[Reap permanents with no attach in N days]' \ + '--keep-max-age[Keep-tag retention for dead sessions (default 7d; 0 = now)]' \ '--fast-fail-window[Fast-fail window (seconds; default 60)]' \ '--fast-fail-limit[Consecutive fast fails before flapping (default 3)]' \ '--print-launchd-plist[Emit a launchd plist that runs pty gc]' \ diff --git a/crates/pty-conformance/src/conformance_map_impl.rs b/crates/pty-conformance/src/conformance_map_impl.rs index 74a4938..6c25beb 100644 --- a/crates/pty-conformance/src/conformance_map_impl.rs +++ b/crates/pty-conformance/src/conformance_map_impl.rs @@ -64,6 +64,7 @@ const SUITES: &[(&str, Kind, &str)] = &[ ("gc-flap-clear-badge-root-len", Kind::Cli, "root-length half in pty_root.rs; badge and restart bookkeeping-strip halves in gc_badge.rs; the flapping machinery itself is dropped in docs/parity.md §12"), ("gc-flapping", Kind::NotPortable, "dropped in docs/parity.md §12 (gc flapping classifier)"), ("gc-generation-guard", Kind::NotPortable, "dropped in docs/parity.md §12 (gc permanent respawn)"), + ("gc-keep-expiry", Kind::Cli, "the keep-tag retention window in the sweep (Node PR #173)"), ("gc-parent-child", Kind::Cli, "the library reapSkipped case (:92) is left out"), ("gc-permanent", Kind::NotPortable, "dropped in docs/parity.md §12 (gc permanent respawn)"), ("gc", Kind::Cli, "debris, orphan tag prune, dry-run, launchd plist (with the plist half of pty-root.test.ts)"), diff --git a/crates/pty-conformance/tests/gc_keep_expiry.rs b/crates/pty-conformance/tests/gc_keep_expiry.rs new file mode 100644 index 0000000..5b96855 --- /dev/null +++ b/crates/pty-conformance/tests/gc_keep_expiry.rs @@ -0,0 +1,224 @@ +//! Port of tests/gc-keep-expiry.test.ts: `keep=true` buys a DEAD session a +//! bounded retention window against `pty gc`, not immortality. Agents tag a +//! session they are debugging right now and never come back to untag it, so +//! an unbounded exemption turns the registry into an append-only log. +//! +//! The policy pinned here: exempt while the session has been dead for less +//! than `--keep-max-age` (default 7d), swept and reported separately once +//! past it, `0` sweeps the whole backlog, and a RUNNING keep session is never +//! a candidate no matter what the flag says. +//! +//! Records are written straight into the root rather than spawned: the policy +//! is a comparison against `exitedAt`/`createdAt`, so a fabricated record is +//! both exact about age and free of daemon-startup waits. + +use pty_conformance::*; + +const DAY: i64 = 24 * 60 * 60; + +/// An exited session, dead for `dead_for_secs`, tagged `keep=true`. +fn write_exited_keep(rig: &Rig, name: &str, dead_for_secs: i64) { + write_fake_metadata( + rig.root(), + name, + FakeMeta::created(-dead_for_secs) + .exited(-dead_for_secs, 0) + .tag("keep", "true"), + ); +} + +/// node: tests/gc-keep-expiry.test.ts:101 +#[test] +fn keeps_an_exited_keep_session_younger_than_the_default_window() { + let rig = Rig::new(); + let name = unique_id("gke"); + write_exited_keep(&rig, &name, 2 * DAY); + + let out = rig.pty(&["gc"]); + expect_status(&out, 0); + let stdout = out.stdout(); + expect_contains(&stdout, &format!("Kept (keep tag): {name}")); + expect_not_contains(&stdout, "keep expired"); + assert!(rig.meta_path(&name).exists()); +} + +/// node: tests/gc-keep-expiry.test.ts:113 +#[test] +fn sweeps_an_expired_keep_session_apart_from_the_plain_sweep() { + let rig = Rig::new(); + let expired = unique_id("gke"); + let stale = unique_id("gke"); + write_exited_keep(&rig, &expired, 30 * DAY); + // A same-age session WITHOUT the tag: proves the two buckets stay + // distinct rather than one absorbing the other. + write_fake_metadata( + rig.root(), + &stale, + FakeMeta::created(-30 * DAY).exited(-30 * DAY, 0), + ); + + let out = rig.pty(&["gc"]); + expect_status(&out, 0); + let stdout = out.stdout(); + expect_contains( + &stdout, + &format!("Removed (keep expired after 7d): {expired}"), + ); + expect_contains(&stdout, &format!("Removed: {stale}")); + expect_contains(&stdout, "1 stale session"); + expect_contains(&stdout, "1 keep-expired session"); + assert!(!rig.meta_path(&expired).exists()); + assert!(!rig.meta_path(&stale).exists()); +} + +/// node: tests/gc-keep-expiry.test.ts:132 +#[test] +fn honours_a_custom_window_in_both_flag_spellings() { + let rig = Rig::new(); + let spaced = unique_id("gke"); + let equals = unique_id("gke"); + write_exited_keep(&rig, &spaced, 2 * 3600); + write_exited_keep(&rig, &equals, 2 * 3600); + + // 3h window: both sessions are 2h dead, so both survive. + let kept = rig.pty(&["gc", "--keep-max-age", "3h"]); + expect_status(&kept, 0); + let stdout = kept.stdout(); + expect_contains(&stdout, &format!("Kept (keep tag): {spaced}")); + expect_contains(&stdout, &format!("Kept (keep tag): {equals}")); + assert!(rig.meta_path(&spaced).exists()); + + // 1h window: both are past it. + let swept = rig.pty(&["gc", "--keep-max-age=1h"]); + expect_status(&swept, 0); + let stdout = swept.stdout(); + expect_contains( + &stdout, + &format!("Removed (keep expired after 1h): {spaced}"), + ); + expect_contains( + &stdout, + &format!("Removed (keep expired after 1h): {equals}"), + ); + assert!(!rig.meta_path(&spaced).exists()); + assert!(!rig.meta_path(&equals).exists()); +} + +/// node: tests/gc-keep-expiry.test.ts:155 +#[test] +fn a_zero_window_sweeps_a_keep_session_that_just_exited() { + let rig = Rig::new(); + let name = unique_id("gke"); + write_exited_keep(&rig, &name, 0); + + let out = rig.pty(&["gc", "--keep-max-age", "0"]); + expect_status(&out, 0); + expect_contains( + &out.stdout(), + &format!("Removed (keep expired after 0s): {name}"), + ); + assert!(!rig.meta_path(&name).exists()); +} + +/// node: tests/gc-keep-expiry.test.ts:166 +#[test] +fn anchors_on_created_at_when_there_is_no_exit_record() { + let rig = Rig::new(); + let name = unique_id("gke"); + // A vanished session (SIGKILLed daemon) never wrote `exitedAt`, so its + // age comes from `createdAt` — the same anchor precedence `pty list + // --older-than` uses. + write_fake_metadata( + rig.root(), + &name, + FakeMeta::created(-30 * DAY).tag("keep", "true"), + ); + + let out = rig.pty(&["gc"]); + expect_status(&out, 0); + expect_contains( + &out.stdout(), + &format!("Removed (keep expired after 7d): {name}"), + ); + assert!(!rig.meta_path(&name).exists()); +} + +/// node: tests/gc-keep-expiry.test.ts:188 +#[test] +fn never_sweeps_a_running_keep_session_even_at_zero() { + let rig = Rig::new(); + let name = unique_id("gke"); + // The test process itself stands in for a live daemon (the same device + // list_filters.rs uses): an alive pid with no exit record reads as + // status=running. Aged well past the window, so the only thing keeping + // it out of the sweep is that it is still running. + std::fs::write(rig.pid_path(&name), std::process::id().to_string()).unwrap(); + write_fake_metadata( + rig.root(), + &name, + FakeMeta::created(-30 * DAY).tag("keep", "true"), + ); + assert_eq!(rig.list_entry(&name).expect("listed")["status"], "running"); + + let out = rig.pty(&["gc", "--keep-max-age", "0"]); + expect_status(&out, 0); + expect_not_contains(&out.stdout(), &name); + assert!(rig.meta_path(&name).exists()); + assert_eq!(rig.list_entry(&name).expect("listed")["status"], "running"); +} + +/// node: tests/gc-keep-expiry.test.ts:214 +#[test] +fn dry_run_previews_keep_expiry_without_removing_anything() { + let rig = Rig::new(); + let name = unique_id("gke"); + write_exited_keep(&rig, &name, 30 * DAY); + + let dry = rig.pty(&["gc", "--dry-run"]); + expect_status(&dry, 0); + let stdout = dry.stdout(); + expect_contains( + &stdout, + &format!("Would remove (keep expired after 7d): {name}"), + ); + expect_contains(&stdout, "1 keep-expired session"); + expect_contains(&stdout, "Dry run"); + assert!(rig.meta_path(&name).exists()); + + // A zero-window dry run is equally non-mutating. + let dry_zero = rig.pty(&["gc", "-n", "--keep-max-age", "0"]); + expect_status(&dry_zero, 0); + expect_contains( + &dry_zero.stdout(), + &format!("Would remove (keep expired after 0s): {name}"), + ); + assert!(rig.meta_path(&name).exists()); + + // And the real pass then actually removes it. + let real = rig.pty(&["gc"]); + expect_status(&real, 0); + expect_contains( + &real.stdout(), + &format!("Removed (keep expired after 7d): {name}"), + ); + assert!(!rig.meta_path(&name).exists()); +} + +/// node: tests/gc-keep-expiry.test.ts:239 +#[test] +fn rejects_a_unit_less_non_zero_window() { + let rig = Rig::new(); + let bare = rig.pty(&["gc", "--keep-max-age", "7"]); + expect_failure(&bare); + expect_contains( + &bare.stderr(), + "--keep-max-age expects a duration like 12h, 7d, or 0", + ); + + let junk = rig.pty(&["gc", "--keep-max-age=soon"]); + expect_failure(&junk); + expect_contains( + &junk.stderr(), + "--keep-max-age expects a duration like 12h, 7d, or 0", + ); +} diff --git a/crates/pty-core/src/registry/mod.rs b/crates/pty-core/src/registry/mod.rs index d9a348e..b0a86ac 100644 --- a/crates/pty-core/src/registry/mod.rs +++ b/crates/pty-core/src/registry/mod.rs @@ -63,9 +63,9 @@ pub use root::{ metadata_path, pid_path, recovery_revision_path, root_length_check, session_dir, socket_path, }; pub use tags::{ - EXACT_RESERVED_TAG_KEYS, GC_BOOKKEEPING_KEYS, KEEP_FALSEY, KEEP_TAG, extract_filter_tags, - is_keep_requested, is_reserved_tag_key, matches_all_tags, reap_on_exit_default, - should_reap_at_exit, strip_gc_bookkeeping, + DEFAULT_KEEP_MAX_AGE_MS, EXACT_RESERVED_TAG_KEYS, GC_BOOKKEEPING_KEYS, KEEP_FALSEY, KEEP_TAG, + extract_filter_tags, is_keep_expired, is_keep_requested, is_reserved_tag_key, matches_all_tags, + reap_on_exit_default, should_reap_at_exit, strip_gc_bookkeeping, }; pub use time::{ iso8601, iso8601_from_epoch_ms, local_hms, now_epoch_ms, now_iso8601, parse_iso8601_ms, diff --git a/crates/pty-core/src/registry/tags.rs b/crates/pty-core/src/registry/tags.rs index 0a9b6b4..528cd17 100644 --- a/crates/pty-core/src/registry/tags.rs +++ b/crates/pty-core/src/registry/tags.rs @@ -2,9 +2,10 @@ //! `--filter-tag` matching, the `keep` tag, exit-time reap precedence, and //! the gc bookkeeping keys a manual restart strips. //! -//! node: src/tags.ts; src/sessions.ts:1020-1097; src/cli.ts:4081-4100 +//! node: src/tags.ts; src/sessions.ts:1020-1109; src/cli.ts:4081-4100 -use super::metadata::TagMap; +use super::metadata::{SessionMetadata, TagMap}; +use super::time::parse_iso8601_ms; /// Keys pty itself treats as bookkeeping and hides from the default /// listing (`pty list --tags` shows them). @@ -54,7 +55,9 @@ pub fn extract_filter_tags(args: &mut Vec) -> Result { Ok(tags) } -/// Tag key that exempts a session from every form of dead-session reaping. +/// Tag key that exempts a session from the daemon's exit-time self-reap +/// unconditionally, and from `pty gc`'s sweep for a bounded window +/// ([`DEFAULT_KEEP_MAX_AGE_MS`]). /// /// node: src/sessions.ts:1020 pub const KEEP_TAG: &str = "keep"; @@ -79,6 +82,47 @@ pub fn is_keep_requested(tags: Option<&TagMap>) -> bool { } } +/// How long `keep` holds a dead session against `pty gc`'s sweep, unless +/// the operator overrides it with `pty gc --keep-max-age `. Seven days +/// is long enough that "I killed it Friday, I'll look Monday" still works, +/// and short enough that a fleet of agents tagging every session cannot +/// grow the registry without bound. +/// +/// node: src/sessions.ts:1084 (`DEFAULT_KEEP_MAX_AGE_MS`) +pub const DEFAULT_KEEP_MAX_AGE_MS: i64 = 7 * 24 * 60 * 60 * 1000; + +/// Has a dead `keep`-tagged session outlived its retention window? +/// +/// Age is anchored on `exitedAt` when the daemon wrote an exit record, else +/// `createdAt` (a `vanished` session never wrote one) — the same anchor +/// precedence `pty list --older-than` uses. Metadata carrying neither, or an +/// unparseable timestamp, has no age and therefore never expires: retaining +/// an unaged record is the recoverable failure, deleting it is not. +/// +/// `max_age_ms <= 0` expires everything, including unaged records — that is +/// the explicit "sweep the keep backlog now" request, not an inference from +/// a timestamp. Callers must apply this to dead sessions only; a running +/// session is never a sweep candidate regardless of its age. +/// +/// node: src/sessions.ts:1098-1109 (`isKeepExpired`) +pub fn is_keep_expired(metadata: Option<&SessionMetadata>, now_ms: i64, max_age_ms: i64) -> bool { + if max_age_ms <= 0 { + return true; + } + let Some(meta) = metadata else { + return false; + }; + let anchor = meta + .exited_at + .as_deref() + .filter(|s| !s.is_empty()) + .unwrap_or(meta.created_at.as_str()); + match parse_iso8601_ms(anchor) { + Some(ts) => now_ms - ts >= max_age_ms, + None => false, + } +} + /// The config default for exit-time reaping: `PTY_REAP_ON_EXIT` unset → /// reap; `false|0|no|off` → preserve; anything else → reap. /// diff --git a/crates/pty-core/tests/registry_tags.rs b/crates/pty-core/tests/registry_tags.rs index 8921e09..19fa0b9 100644 --- a/crates/pty-core/tests/registry_tags.rs +++ b/crates/pty-core/tests/registry_tags.rs @@ -1,7 +1,8 @@ //! Tag rules: reserved keys, filter matching, `--filter-tag` extraction, the -//! `keep` tag, exit-time reap precedence, and gc bookkeeping stripping. +//! `keep` tag and its retention window, exit-time reap precedence, and gc +//! bookkeeping stripping. -use pty_core::registry::{self, TagMap}; +use pty_core::registry::{self, SessionMetadata, TagMap}; fn tags(pairs: &[(&str, &str)]) -> TagMap { pairs @@ -118,6 +119,61 @@ fn keep_tag_semantics() { } } +/// The retention window that bounds `keep` in `pty gc`'s sweep: the anchor +/// precedence, the record that has no age, and the zero that sweeps +/// everything. +/// +/// node: src/sessions.ts:1084-1109 +#[test] +fn keep_expiry_window() { + let day = 24 * 60 * 60 * 1000; + assert_eq!(registry::DEFAULT_KEEP_MAX_AGE_MS, 7 * day); + let now = registry::parse_iso8601_ms("2026-09-04T12:00:00.000Z").unwrap(); + let at = |offset_days: i64| registry::iso8601_from_epoch_ms(now - offset_days * day); + + // `exitedAt` wins over `createdAt`: created long ago, died just now. + let fresh_exit = SessionMetadata { + created_at: at(30), + exited_at: Some(at(2)), + ..Default::default() + }; + assert!(!registry::is_keep_expired(Some(&fresh_exit), now, 7 * day)); + assert!(registry::is_keep_expired(Some(&fresh_exit), now, day)); + + // No exit record (a vanished session): `createdAt` is the anchor. + let vanished = SessionMetadata { + created_at: at(30), + ..Default::default() + }; + assert!(registry::is_keep_expired(Some(&vanished), now, 7 * day)); + + // Neither timestamp, or an unparseable one: no age, so never expired — + // retaining an unaged record is the recoverable failure. + let unaged = SessionMetadata::default(); + assert!(!registry::is_keep_expired(Some(&unaged), now, 7 * day)); + let bogus = SessionMetadata { + created_at: "not a timestamp".into(), + ..Default::default() + }; + assert!(!registry::is_keep_expired(Some(&bogus), now, 7 * day)); + assert!(!registry::is_keep_expired(None, now, 7 * day)); + + // Zero is the explicit "sweep the backlog now": every record expires, + // including the unaged ones and a session that died this instant. + for meta in [&fresh_exit, &vanished, &unaged, &bogus] { + assert!(registry::is_keep_expired(Some(meta), now, 0)); + } + assert!(registry::is_keep_expired(None, now, 0)); + + // The boundary is inclusive: exactly the window old is expired. + let exactly = SessionMetadata { + created_at: at(0), + exited_at: Some(at(7)), + ..Default::default() + }; + assert!(registry::is_keep_expired(Some(&exactly), now, 7 * day)); +} + /// node: src/sessions.ts:1069-1089 #[test] fn should_reap_at_exit_precedence() { diff --git a/crates/pty/src/cli/gc.rs b/crates/pty/src/cli/gc.rs index 89bcbc2..c5835b5 100644 --- a/crates/pty/src/cli/gc.rs +++ b/crates/pty/src/cli/gc.rs @@ -1,6 +1,7 @@ -//! `pty gc [-n|--dry-run]`: reclaim registry debris, kill orphaned -//! `parent=` children, sweep exited/vanished sessions (honouring the -//! `keep` tag), and prune dead `:l-` layout tags. +//! `pty gc [-n|--dry-run] [--keep-max-age ]`: reclaim registry debris, +//! kill orphaned `parent=` children, sweep exited/vanished sessions +//! (honouring the `keep` tag until it expires), and prune dead +//! `:l-` layout tags. //! `pty gc --print-launchd-plist [--interval N]` prints a launchd job. //! //! The permanent-respawn, flapping and abandoned-reap steps of Node's gc @@ -8,22 +9,23 @@ //! dead session out of the sweep. `--idle-days` and `--fast-fail-*` belonged //! to those steps: they are accepted and ignored. //! -//! node: src/cli.ts:1411-1453 (parsing), 3089-3202 (`cmdGc`), 3224-3276 +//! node: src/cli.ts:1450-1500 (parsing), 3185-3316 (`cmdGc`), 3338-3390 //! (`printLaunchdPlist`); src/sessions.ts:620-880 (raw debris, observed -//! cleanup, `reapObservedSession`), 1521-1724 (`gc`), 2026-2075 -//! (`pruneOrphanLayoutTags`) +//! cleanup, `reapObservedSession`), 1084-1109 (the keep window), 1596-1806 +//! (`gc`), 2140-2189 (`pruneOrphanLayoutTags`) use std::collections::{BTreeSet, HashMap}; use std::io::Write; use std::path::PathBuf; use std::time::{Duration, Instant}; +use pty_core::duration::{format_duration, parse_duration}; use pty_core::registry::{ - self, DEFAULT_SOCKET_PROBE_BUDGET, SessionInfo, TagMap, cleanup_all_while_locked, - cleanup_socket, default_session_dir, events_path, has_process_exited_for_reap, - is_keep_requested, metadata_matches_observation, metadata_path, pid_alive, - probe_sockets_within_budget, read_metadata, read_pid, read_pid_with, recovery_revision_path, - session_dir, socket_path, update_tags, with_both_locks, + self, DEFAULT_KEEP_MAX_AGE_MS, DEFAULT_SOCKET_PROBE_BUDGET, SessionInfo, TagMap, + cleanup_all_while_locked, cleanup_socket, default_session_dir, events_path, + has_process_exited_for_reap, is_keep_expired, is_keep_requested, metadata_matches_observation, + metadata_path, now_epoch_ms, pid_alive, probe_sockets_within_budget, read_metadata, read_pid, + read_pid_with, recovery_revision_path, session_dir, socket_path, update_tags, with_both_locks, }; use super::argv::js_parse_int; @@ -36,6 +38,7 @@ pub fn run(gc_args: &[String]) -> CliResult { let dry_run = gc_args.iter().any(|a| a == "--dry-run" || a == "-n"); let print_plist = gc_args.iter().any(|a| a == "--print-launchd-plist"); let mut interval: i64 = 30; + let mut keep_max_age_ms = DEFAULT_KEEP_MAX_AGE_MS; let parse_positive = |flag: &str, raw: &str| -> Result { match js_parse_int(raw) { Some(v) if v > 0 => Ok(v), @@ -44,6 +47,20 @@ pub fn run(gc_args: &[String]) -> CliResult { ))), } }; + // Durations, unlike the integer flags, have a meaningful zero: `0` means + // "the keep exemption is over, sweep the backlog now". The unit-less + // spelling is accepted only for zero, since `--keep-max-age 7` would + // otherwise be ambiguous between seconds and days. + let parse_age = |flag: &str, raw: &str| -> Result { + if raw.trim() == "0" { + return Ok(0); + } + parse_duration(raw).ok_or_else(|| { + CliError(format!( + "pty gc: {flag} expects a duration like 12h, 7d, or 0 (got \"{raw}\")" + )) + }) + }; // The dropped tuning flags: consumed with their value, never validated. const IGNORED: [&str; 3] = ["--idle-days", "--fast-fail-window", "--fast-fail-limit"]; let mut i = 0; @@ -54,6 +71,11 @@ pub fn run(gc_args: &[String]) -> CliResult { interval = parse_positive("--interval", &gc_args[i])?; } else if let Some(raw) = a.strip_prefix("--interval=") { interval = parse_positive("--interval", raw)?; + } else if a == "--keep-max-age" && i + 1 < gc_args.len() { + i += 1; + keep_max_age_ms = parse_age("--keep-max-age", &gc_args[i])?; + } else if let Some(raw) = a.strip_prefix("--keep-max-age=") { + keep_max_age_ms = parse_age("--keep-max-age", raw)?; } else if IGNORED.contains(&a) && i + 1 < gc_args.len() { i += 1; } @@ -63,7 +85,7 @@ pub fn run(gc_args: &[String]) -> CliResult { print_launchd_plist(interval); return Ok(0); } - cmd_gc(dry_run) + cmd_gc(dry_run, keep_max_age_ms) } /// What one pass did (or would do). @@ -71,6 +93,10 @@ pub fn run(gc_args: &[String]) -> CliResult { struct GcResult { removed: Vec, kept: Vec, + /// Dead sessions swept despite a `keep` tag because they outlived the + /// retention window. Disjoint from `removed`, which holds the untagged + /// sweep, so the two reasons are reported apart. + keep_expired: Vec, killed_orphan_children: Vec, reap_skipped: Vec, } @@ -99,9 +125,9 @@ struct PrunedTags { /// `cmdGc`. /// -/// node: src/cli.ts:3089-3202 -fn cmd_gc(dry_run: bool) -> CliResult { - let result = gc(dry_run); +/// node: src/cli.ts:3185-3316 +fn cmd_gc(dry_run: bool, keep_max_age_ms: i64) -> CliResult { + let result = gc(dry_run, keep_max_age_ms); let pruned = prune_orphan_layout_tags(dry_run); let killed_verb = if dry_run { "Would kill orphan child" } else { "Killed orphan child" }; @@ -121,10 +147,21 @@ fn cmd_gc(dry_run: bool) -> CliResult { for name in &result.removed { println!("{remove_verb}: {name}"); } + // Reported apart from the plain sweep above: an operator who tagged + // these sessions asked for them to survive, so the reason they went away + // anyway has to be visible rather than looking like the keep tag was + // ignored. + let window = format_duration(keep_max_age_ms); + for name in &result.keep_expired { + println!("{remove_verb} (keep expired after {window}): {name}"); + } // A kept session is not an action; it is printed so "why is this dead - // session still listed?" has a visible answer. + // session still listed?" has a visible answer, naming the window it is + // counting down. for name in &result.kept { - println!("Kept (keep tag): {name} — remove the keep tag to reap it"); + println!( + "Kept (keep tag): {name} — swept once dead for {window}, or remove the keep tag to reap it now" + ); } for p in &pruned { println!( @@ -142,6 +179,7 @@ fn cmd_gc(dry_run: bool) -> CliResult { let total_actions = result.killed_orphan_children.len() + result.reap_skipped.len() + result.removed.len() + + result.keep_expired.len() + total_tags; if total_actions == 0 { println!( @@ -165,6 +203,10 @@ fn cmd_gc(dry_run: bool) -> CliResult { if n > 0 { parts.push(format!("{n} stale {}", plural(n, "session", "sessions"))); } + let n = result.keep_expired.len(); + if n > 0 { + parts.push(format!("{n} keep-expired {}", plural(n, "session", "sessions"))); + } if total_tags > 0 { parts.push(format!( "{total_tags} orphan {}", @@ -181,8 +223,8 @@ fn cmd_gc(dry_run: bool) -> CliResult { /// The pass. /// -/// node: src/sessions.ts:1521-1724 -fn gc(dry_run: bool) -> GcResult { +/// node: src/sessions.ts:1596-1806 +fn gc(dry_run: bool, keep_max_age_ms: i64) -> GcResult { let mut result = GcResult::default(); // Raw debris: runtime files whose metadata is missing or malformed. @@ -235,8 +277,10 @@ fn gc(dry_run: bool) -> GcResult { } // STEP 3: the historic sweep. Exited/vanished non-permanent sessions - // lose their metadata; `keep` exempts. + // lose their metadata; `keep` exempts them, but only until they have + // been dead longer than the retention window. let final_list = if dry_run { initial } else { registry::list_sessions() }; + let now_ms = now_epoch_ms(); for s in &final_list { if !s.is_gone() { continue; @@ -245,12 +289,18 @@ fn gc(dry_run: bool) -> GcResult { if tags.and_then(|t| t.get("strategy")).map(String::as_str) == Some("permanent") { continue; } - if is_keep_requested(tags) { + let keep_requested = is_keep_requested(tags); + if keep_requested && !is_keep_expired(s.metadata.as_ref(), now_ms, keep_max_age_ms) { result.kept.push(s.name.clone()); continue; } if dry_run || cleanup_observed_session(s) { - result.removed.push(s.name.clone()); + let bucket = if keep_requested { + &mut result.keep_expired + } else { + &mut result.removed + }; + bucket.push(s.name.clone()); } } result diff --git a/crates/pty/tests/cli_gc.rs b/crates/pty/tests/cli_gc.rs index 27a55de..21a36ae 100644 --- a/crates/pty/tests/cli_gc.rs +++ b/crates/pty/tests/cli_gc.rs @@ -22,22 +22,22 @@ fn sweeps_gone_sessions_and_keeps_kept_ones() { let out = rig.ok(&["gc", "-n"]); assert_eq!( out.stdout, - "Would remove: ex\nWould remove: van\nKept (keep tag): kept — remove the keep tag to reap it\nWould clean up 2 stale sessions. (Dry run — no changes made.)\n" + "Would remove: ex\nWould remove: van\nKept (keep tag): kept — swept once dead for 7d, or remove the keep tag to reap it now\nWould clean up 2 stale sessions. (Dry run — no changes made.)\n" ); assert!(rig.exists("van.json") && rig.exists("ex.json")); let out = rig.ok(&["gc"]); assert_eq!( out.stdout, - "Removed: ex\nRemoved: van\nKept (keep tag): kept — remove the keep tag to reap it\nCleaned up 2 stale sessions.\n" + "Removed: ex\nRemoved: van\nKept (keep tag): kept — swept once dead for 7d, or remove the keep tag to reap it now\nCleaned up 2 stale sessions.\n" ); assert!(!rig.exists("van.json") && !rig.exists("ex.json")); assert!(rig.exists("kept.json") && rig.exists("perm.json")); let out = rig.ok(&["gc"]); - assert_eq!(out.stdout, "Kept (keep tag): kept — remove the keep tag to reap it\nNothing to clean up.\n"); + assert_eq!(out.stdout, "Kept (keep tag): kept — swept once dead for 7d, or remove the keep tag to reap it now\nNothing to clean up.\n"); rig.write_meta("one", json!({})); assert_eq!( rig.ok(&["gc"]).stdout, - "Removed: one\nKept (keep tag): kept — remove the keep tag to reap it\nCleaned up 1 stale session.\n" + "Removed: one\nKept (keep tag): kept — swept once dead for 7d, or remove the keep tag to reap it now\nCleaned up 1 stale session.\n" ); } @@ -66,7 +66,7 @@ fn kills_orphan_children() { let out = rig.ok(&["gc", "--dry-run"]); assert_eq!( out.stdout, - "Would kill orphan child: child-dead (parent dead-parent dead)\nWould kill orphan child: child-missing (parent nonexistent-parent missing)\nWould remove: child-dead\nWould remove: child-missing\nKept (keep tag): dead-parent — remove the keep tag to reap it\nWould clean up 2 orphan children, 2 stale sessions. (Dry run — no changes made.)\n" + "Would kill orphan child: child-dead (parent dead-parent dead)\nWould kill orphan child: child-missing (parent nonexistent-parent missing)\nWould remove: child-dead\nWould remove: child-missing\nKept (keep tag): dead-parent — swept once dead for 7d, or remove the keep tag to reap it now\nWould clean up 2 orphan children, 2 stale sessions. (Dry run — no changes made.)\n" ); assert!(rig.exists("child-dead.json")); @@ -76,7 +76,7 @@ fn kills_orphan_children() { let out = rig.ok(&["gc"]); assert_eq!( out.stdout, - "Killed orphan child: child-dead (parent dead-parent dead)\nKilled orphan child: child-missing (parent nonexistent-parent missing)\nKept (keep tag): dead-parent — remove the keep tag to reap it\nCleaned up 2 orphan children.\n" + "Killed orphan child: child-dead (parent dead-parent dead)\nKilled orphan child: child-missing (parent nonexistent-parent missing)\nKept (keep tag): dead-parent — swept once dead for 7d, or remove the keep tag to reap it now\nCleaned up 2 orphan children.\n" ); assert!(!rig.exists("child-dead.json") && !rig.exists("child-missing.json")); assert!(rig.exists("happy-child.json") && rig.exists("live-parent.json")); diff --git a/crates/pty/tests/fixtures/help/gc.txt b/crates/pty/tests/fixtures/help/gc.txt index f6cdbed..06dc8ad 100644 --- a/crates/pty/tests/fixtures/help/gc.txt +++ b/crates/pty/tests/fixtures/help/gc.txt @@ -1,4 +1,4 @@ -Usage: pty gc [-n] [--idle-days N] [--fast-fail-window=N] [--fast-fail-limit=N] +Usage: pty gc [-n] [--idle-days N] [--keep-max-age ] [--fast-fail-window=N] [--fast-fail-limit=N] pty gc --print-launchd-plist [--interval=N] One reconciliation pass: sweep exited/vanished, orphan-kill `parent=` children, @@ -6,11 +6,14 @@ reap abandoned permanents, respawn `strategy=permanent` sessions. Non-permanent sessions remove themselves as they exit, so the sweep is a backstop: it mainly catches `vanished` sessions, whose daemon was killed outright and so -never ran its own cleanup. Sessions tagged `keep` are never swept. +never ran its own cleanup. A session tagged `keep` is swept only once it has been +dead longer than --keep-max-age; running sessions are never swept. Flags: -n, --dry-run Preview without changing anything --idle-days N Also reap permanents with no attach in N days + --keep-max-age How long `keep` holds a DEAD session against the sweep + (default 7d; 0 sweeps every dead keep session now) --fast-fail-window=N Fast-fail window seconds (default 60; per-session tag wins) --fast-fail-limit=N Consecutive fast fails before flapping (default 3; per-session tag wins) --print-launchd-plist Print a macOS launchd plist that runs 'pty gc' on an interval @@ -18,4 +21,5 @@ Flags: Examples: pty gc --dry-run + pty gc --keep-max-age 0 pty gc --print-launchd-plist > ~/Library/LaunchAgents/com.compoundingtech.pty.gc.plist diff --git a/crates/pty/tests/fixtures/help/usage.txt b/crates/pty/tests/fixtures/help/usage.txt index 0b3a39f..84d538a 100644 --- a/crates/pty/tests/fixtures/help/usage.txt +++ b/crates/pty/tests/fixtures/help/usage.txt @@ -86,6 +86,8 @@ Lifecycle: permanent-respawn, exited-sweep pty gc --dry-run Preview without changing anything (alias: -n) pty gc --idle-days N Also reap permanents with no attach in N days + pty gc --keep-max-age How long a `keep` tag holds a DEAD session against + the sweep (default 7d; 0 sweeps the backlog now) pty gc --fast-fail-window=N Fast-fail window (seconds) for the respawn cap (default 60; per-session strategy.fast-fail-window wins) pty gc --fast-fail-limit=N Consecutive fast fails before a permanent is flagged diff --git a/docs/conformance.md b/docs/conformance.md index 62ea766..e992097 100644 --- a/docs/conformance.md +++ b/docs/conformance.md @@ -8,10 +8,10 @@ Run: `PTY_TEST_BIN=$(which pty) cargo test -p pty-conformance` (Node) and `cargo ## Summary -- Node suites: 120 -- Rust conformance tests: 648 (627 port a Node test, 21 cover the Rust-owned fixtures) +- Node suites: 121 +- Rust conformance tests: 660 (635 port a Node test, 25 cover the Rust-owned fixtures) - Gated (`_node`/`_rust` pairs pointing at a decision record): 6 — the parity debt -- cli: 55 suites, 55 with Rust tests, 0 to do +- cli: 56 suites, 56 with Rust tests, 0 to do - not-portable: 49 - protocol: 9 suites, 9 with Rust tests, 0 to do - unit: 7 @@ -51,6 +51,7 @@ Run: `PTY_TEST_BIN=$(which pty) cargo test -p pty-conformance` (Node) and `cargo | gc-flap-clear-badge-root-len.test.ts | cli | gc_badge.rs, pty_root.rs | 7 | 84, 118, 142, 163, 182, 197, 217 — root-length half in pty_root.rs; badge and restart bookkeeping-strip halves in gc_badge.rs; the flapping machinery itself is dropped in docs/parity.md §12 | | gc-flapping.test.ts | not-portable | — | 0 | dropped in docs/parity.md §12 (gc flapping classifier) | | gc-generation-guard.test.ts | not-portable | — | 0 | dropped in docs/parity.md §12 (gc permanent respawn) | +| gc-keep-expiry.test.ts | cli | gc_keep_expiry.rs | 8 | 101, 113, 132, 155, 166, 188, 214, 239 — the keep-tag retention window in the sweep (Node PR #173) | | gc-parent-child.test.ts | cli | gc_parent_child.rs | 6 | 121, 140, 153, 169, 192, 213 — the library reapSkipped case (:92) is left out | | gc-permanent.test.ts | not-portable | — | 0 | dropped in docs/parity.md §12 (gc permanent respawn) | | help.test.ts | cli | help.rs | 2 | 104 — top-level usage only; per-command help belongs to the help work package | diff --git a/docs/parity.md b/docs/parity.md index 72432e3..cd60328 100644 --- a/docs/parity.md +++ b/docs/parity.md @@ -330,7 +330,7 @@ Decided on 2026-08-29. Each row records the decision. | `pty-kill-releases-socket-test` second binary | S | Dropped. The case becomes a Rust test. | | `remote-serve --socket ` | S | Dropped. `--stdio` stays. The Node docs mark the socket form transitional. **`pty remote-serve --help` still describes `--socket`**, because the help texts are vendored from the Node tool byte for byte so the help test can compare them. Passing `--socket` prints a usage line naming only `--stdio` and exits 1. Checked 2026-09-02. | | Legacy positional display name (`pty run mylabel -- cmd`) and the `Hint:` line | S | Dropped. Nothing in the network uses it. | -| `gc`: permanent respawn, flapping classifier, abandoned reap | L | Dropped. `st2` supervises agents now. Node PR #60 (July, held) planned this removal. Kept: debris, orphan kill, sweep, `keep`, tag prune, dry-run, footer, `--print-launchd-plist`. `strategy=permanent` stays as a preserve flag. Their tuning flags `--idle-days`, `--fast-fail-window`, `--fast-fail-limit` (both `--flag N` and `--flag=N`) are accepted with their value and ignored, never rejected, so scripts written for Node keep working. | +| `gc`: permanent respawn, flapping classifier, abandoned reap | L | Dropped. `st2` supervises agents now. Node PR #60 (July, held) planned this removal. Kept: debris, orphan kill, sweep, `keep` and its `--keep-max-age` retention window, tag prune, dry-run, footer, `--print-launchd-plist`. `strategy=permanent` stays as a preserve flag. Their tuning flags `--idle-days`, `--fast-fail-window`, `--fast-fail-limit` (both `--flag N` and `--flag=N`) are accepted with their value and ignored, never rejected, so scripts written for Node keep working. | | `recover` and the `recovery{}` capability | XL | Deferred and documented as absent. No program in the network calls it. Rust daemons omit the capability; Node `list` handles that. Rust preserves the field on rewrite, so `recovery.metadataRevision` goes stale for a session a Rust binary writes to — accepted, decision 0005. | | `evidence snapshot` / `remove` | M | Deferred and documented as absent. Its user is not known. | | `--attach-stream-fd-v1` | M | Kept. An eval cell and relays use it. | @@ -446,6 +446,7 @@ Checked again on 2026-09-02. | Where | What | Where it stands | |---|---|---| | Node PR #168 | Persist `lastOutputAtMs`, the time the child last printed | **Merged 2026-08-29, the day this plan was approved, and nobody noticed.** Now ported: the daemon stamps it, persists it at most once a second, and carries it into the exit record. See `crates/pty-conformance/tests/output_activity.rs`. | +| Node PR #173 | Bounded `keep` retention in gc: `--keep-max-age ` (default `7d`, `0` sweeps now), keep-expired reported apart from the plain sweep | **Merged 2026-09-04**, and ported the same day: `pty gc --keep-max-age`, `registry::is_keep_expired` / `DEFAULT_KEEP_MAX_AGE_MS`, `crates/pty-conformance/tests/gc_keep_expiry.rs`. Node's own suite cannot be cross-run here yet — the installed Node binary is 0.12.0, which predates the flag. | | Node PRs #131, #133 | Generation-bound activity status; revision-guarded send | Both still drafts. Watch. | | Node PR #60 | Lean core: delete `up`/`down`, gc respawn, flapping | Still held. Superseded by `st2`. Informs section 12. | | Node issue #167 | `--isolate-env` drops `TERM_PROGRAM` and `GHOSTTY_*`, so a full-screen program cannot tell what terminal it is in | Open. This port carries the same allow-list, so it has the same problem. Fixing it means changing both. |