Skip to content

Commit 8876a2a

Browse files
feat(gc): expire keep-tagged exited sessions after a max age
`keep=true` bought a dead session immortality against `pty gc`'s sweep. Agents tag the session they are debugging right now and never come back to untag it, so the exemption turned the registry into an append-only log (740 of 911 sessions on one host, upstream). `pty gc --keep-max-age <dur>` (default `7d`, `0` sweeps the dead keep backlog on this pass) now bounds it. Only exited/vanished sessions are candidates — a running session is never swept whatever its age. Age anchors on `exitedAt`, falling back to `createdAt` for a `vanished` session that never wrote one, the same precedence `pty list --older-than` uses; a record with neither never expires except under `0`. A unit-less non-zero value is rejected rather than guessed at, since `--keep-max-age 7` is ambiguous between seconds and days. Swept keep sessions are reported apart from the plain sweep — `Removed (keep expired after 7d): <name>` and an `N keep-expired sessions` term in the footer — so the reason is visible instead of looking like the tag was ignored, and the retained line now names the window it is counting down. Port of compoundingtech/pty#173 (merged 86dcc5eb7): same flag name, default, duration grammar, and output wording, with the help text and completions vendored from it byte for byte. agent-identity: dev3.direct.omp.v6c4mkm2 agent-persona: generalist agent-supervisor: unavailable agent-tool: OMP agent-tool-version: 18.1.2 agent-runtime: OMP 18.1.2 tooling-profile: dotfiles@7534055
1 parent a8e9a3e commit 8876a2a

14 files changed

Lines changed: 428 additions & 43 deletions

File tree

completions/pty.bash

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -95,7 +95,7 @@ _pty() {
9595
fi
9696
;;
9797
gc)
98-
COMPREPLY=($(compgen -W "-n --dry-run --idle-days --fast-fail-window --fast-fail-limit --print-launchd-plist --interval" -- "${cur}"))
98+
COMPREPLY=($(compgen -W "-n --dry-run --idle-days --keep-max-age --fast-fail-window --fast-fail-limit --print-launchd-plist --interval" -- "${cur}"))
9999
;;
100100
tag)
101101
if [[ "${cur}" == -* ]]; then

completions/pty.fish

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -133,6 +133,7 @@ complete -c pty -n '__pty_using_command recover' -l snapshot -d 'Captured capabi
133133
complete -c pty -n '__pty_using_command rm remove' -a '(__pty_sessions)' -d 'Session'
134134
complete -c pty -n '__pty_using_command gc' -l dry-run -s n -d 'Preview without changing anything'
135135
complete -c pty -n '__pty_using_command gc' -l idle-days -d 'Reap permanents with no attach in N days'
136+
complete -c pty -n '__pty_using_command gc' -l keep-max-age -d 'Keep-tag retention for dead sessions (default 7d; 0 = now)'
136137
complete -c pty -n '__pty_using_command gc' -l fast-fail-window -d 'Fast-fail window (seconds; default 60)'
137138
complete -c pty -n '__pty_using_command gc' -l fast-fail-limit -d 'Consecutive fast fails before flapping (default 3)'
138139
complete -c pty -n '__pty_using_command gc' -l print-launchd-plist -d 'Emit a launchd plist that runs pty gc'

completions/pty.zsh

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -149,6 +149,7 @@ _pty() {
149149
_arguments \
150150
'(n --dry-run){n,--dry-run}[Preview without changing anything]' \
151151
'--idle-days[Reap permanents with no attach in N days]' \
152+
'--keep-max-age[Keep-tag retention for dead sessions (default 7d; 0 = now)]' \
152153
'--fast-fail-window[Fast-fail window (seconds; default 60)]' \
153154
'--fast-fail-limit[Consecutive fast fails before flapping (default 3)]' \
154155
'--print-launchd-plist[Emit a launchd plist that runs pty gc]' \

crates/pty-conformance/src/conformance_map_impl.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -64,6 +64,7 @@ const SUITES: &[(&str, Kind, &str)] = &[
6464
("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"),
6565
("gc-flapping", Kind::NotPortable, "dropped in docs/parity.md §12 (gc flapping classifier)"),
6666
("gc-generation-guard", Kind::NotPortable, "dropped in docs/parity.md §12 (gc permanent respawn)"),
67+
("gc-keep-expiry", Kind::Cli, "the keep-tag retention window in the sweep (Node PR #173)"),
6768
("gc-parent-child", Kind::Cli, "the library reapSkipped case (:92) is left out"),
6869
("gc-permanent", Kind::NotPortable, "dropped in docs/parity.md §12 (gc permanent respawn)"),
6970
("gc", Kind::Cli, "debris, orphan tag prune, dry-run, launchd plist (with the plist half of pty-root.test.ts)"),
Lines changed: 224 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,224 @@
1+
//! Port of tests/gc-keep-expiry.test.ts: `keep=true` buys a DEAD session a
2+
//! bounded retention window against `pty gc`, not immortality. Agents tag a
3+
//! session they are debugging right now and never come back to untag it, so
4+
//! an unbounded exemption turns the registry into an append-only log.
5+
//!
6+
//! The policy pinned here: exempt while the session has been dead for less
7+
//! than `--keep-max-age` (default 7d), swept and reported separately once
8+
//! past it, `0` sweeps the whole backlog, and a RUNNING keep session is never
9+
//! a candidate no matter what the flag says.
10+
//!
11+
//! Records are written straight into the root rather than spawned: the policy
12+
//! is a comparison against `exitedAt`/`createdAt`, so a fabricated record is
13+
//! both exact about age and free of daemon-startup waits.
14+
15+
use pty_conformance::*;
16+
17+
const DAY: i64 = 24 * 60 * 60;
18+
19+
/// An exited session, dead for `dead_for_secs`, tagged `keep=true`.
20+
fn write_exited_keep(rig: &Rig, name: &str, dead_for_secs: i64) {
21+
write_fake_metadata(
22+
rig.root(),
23+
name,
24+
FakeMeta::created(-dead_for_secs)
25+
.exited(-dead_for_secs, 0)
26+
.tag("keep", "true"),
27+
);
28+
}
29+
30+
/// node: tests/gc-keep-expiry.test.ts:101
31+
#[test]
32+
fn keeps_an_exited_keep_session_younger_than_the_default_window() {
33+
let rig = Rig::new();
34+
let name = unique_id("gke");
35+
write_exited_keep(&rig, &name, 2 * DAY);
36+
37+
let out = rig.pty(&["gc"]);
38+
expect_status(&out, 0);
39+
let stdout = out.stdout();
40+
expect_contains(&stdout, &format!("Kept (keep tag): {name}"));
41+
expect_not_contains(&stdout, "keep expired");
42+
assert!(rig.meta_path(&name).exists());
43+
}
44+
45+
/// node: tests/gc-keep-expiry.test.ts:113
46+
#[test]
47+
fn sweeps_an_expired_keep_session_apart_from_the_plain_sweep() {
48+
let rig = Rig::new();
49+
let expired = unique_id("gke");
50+
let stale = unique_id("gke");
51+
write_exited_keep(&rig, &expired, 30 * DAY);
52+
// A same-age session WITHOUT the tag: proves the two buckets stay
53+
// distinct rather than one absorbing the other.
54+
write_fake_metadata(
55+
rig.root(),
56+
&stale,
57+
FakeMeta::created(-30 * DAY).exited(-30 * DAY, 0),
58+
);
59+
60+
let out = rig.pty(&["gc"]);
61+
expect_status(&out, 0);
62+
let stdout = out.stdout();
63+
expect_contains(
64+
&stdout,
65+
&format!("Removed (keep expired after 7d): {expired}"),
66+
);
67+
expect_contains(&stdout, &format!("Removed: {stale}"));
68+
expect_contains(&stdout, "1 stale session");
69+
expect_contains(&stdout, "1 keep-expired session");
70+
assert!(!rig.meta_path(&expired).exists());
71+
assert!(!rig.meta_path(&stale).exists());
72+
}
73+
74+
/// node: tests/gc-keep-expiry.test.ts:132
75+
#[test]
76+
fn honours_a_custom_window_in_both_flag_spellings() {
77+
let rig = Rig::new();
78+
let spaced = unique_id("gke");
79+
let equals = unique_id("gke");
80+
write_exited_keep(&rig, &spaced, 2 * 3600);
81+
write_exited_keep(&rig, &equals, 2 * 3600);
82+
83+
// 3h window: both sessions are 2h dead, so both survive.
84+
let kept = rig.pty(&["gc", "--keep-max-age", "3h"]);
85+
expect_status(&kept, 0);
86+
let stdout = kept.stdout();
87+
expect_contains(&stdout, &format!("Kept (keep tag): {spaced}"));
88+
expect_contains(&stdout, &format!("Kept (keep tag): {equals}"));
89+
assert!(rig.meta_path(&spaced).exists());
90+
91+
// 1h window: both are past it.
92+
let swept = rig.pty(&["gc", "--keep-max-age=1h"]);
93+
expect_status(&swept, 0);
94+
let stdout = swept.stdout();
95+
expect_contains(
96+
&stdout,
97+
&format!("Removed (keep expired after 1h): {spaced}"),
98+
);
99+
expect_contains(
100+
&stdout,
101+
&format!("Removed (keep expired after 1h): {equals}"),
102+
);
103+
assert!(!rig.meta_path(&spaced).exists());
104+
assert!(!rig.meta_path(&equals).exists());
105+
}
106+
107+
/// node: tests/gc-keep-expiry.test.ts:155
108+
#[test]
109+
fn a_zero_window_sweeps_a_keep_session_that_just_exited() {
110+
let rig = Rig::new();
111+
let name = unique_id("gke");
112+
write_exited_keep(&rig, &name, 0);
113+
114+
let out = rig.pty(&["gc", "--keep-max-age", "0"]);
115+
expect_status(&out, 0);
116+
expect_contains(
117+
&out.stdout(),
118+
&format!("Removed (keep expired after 0s): {name}"),
119+
);
120+
assert!(!rig.meta_path(&name).exists());
121+
}
122+
123+
/// node: tests/gc-keep-expiry.test.ts:166
124+
#[test]
125+
fn anchors_on_created_at_when_there_is_no_exit_record() {
126+
let rig = Rig::new();
127+
let name = unique_id("gke");
128+
// A vanished session (SIGKILLed daemon) never wrote `exitedAt`, so its
129+
// age comes from `createdAt` — the same anchor precedence `pty list
130+
// --older-than` uses.
131+
write_fake_metadata(
132+
rig.root(),
133+
&name,
134+
FakeMeta::created(-30 * DAY).tag("keep", "true"),
135+
);
136+
137+
let out = rig.pty(&["gc"]);
138+
expect_status(&out, 0);
139+
expect_contains(
140+
&out.stdout(),
141+
&format!("Removed (keep expired after 7d): {name}"),
142+
);
143+
assert!(!rig.meta_path(&name).exists());
144+
}
145+
146+
/// node: tests/gc-keep-expiry.test.ts:188
147+
#[test]
148+
fn never_sweeps_a_running_keep_session_even_at_zero() {
149+
let rig = Rig::new();
150+
let name = unique_id("gke");
151+
// The test process itself stands in for a live daemon (the same device
152+
// list_filters.rs uses): an alive pid with no exit record reads as
153+
// status=running. Aged well past the window, so the only thing keeping
154+
// it out of the sweep is that it is still running.
155+
std::fs::write(rig.pid_path(&name), std::process::id().to_string()).unwrap();
156+
write_fake_metadata(
157+
rig.root(),
158+
&name,
159+
FakeMeta::created(-30 * DAY).tag("keep", "true"),
160+
);
161+
assert_eq!(rig.list_entry(&name).expect("listed")["status"], "running");
162+
163+
let out = rig.pty(&["gc", "--keep-max-age", "0"]);
164+
expect_status(&out, 0);
165+
expect_not_contains(&out.stdout(), &name);
166+
assert!(rig.meta_path(&name).exists());
167+
assert_eq!(rig.list_entry(&name).expect("listed")["status"], "running");
168+
}
169+
170+
/// node: tests/gc-keep-expiry.test.ts:214
171+
#[test]
172+
fn dry_run_previews_keep_expiry_without_removing_anything() {
173+
let rig = Rig::new();
174+
let name = unique_id("gke");
175+
write_exited_keep(&rig, &name, 30 * DAY);
176+
177+
let dry = rig.pty(&["gc", "--dry-run"]);
178+
expect_status(&dry, 0);
179+
let stdout = dry.stdout();
180+
expect_contains(
181+
&stdout,
182+
&format!("Would remove (keep expired after 7d): {name}"),
183+
);
184+
expect_contains(&stdout, "1 keep-expired session");
185+
expect_contains(&stdout, "Dry run");
186+
assert!(rig.meta_path(&name).exists());
187+
188+
// A zero-window dry run is equally non-mutating.
189+
let dry_zero = rig.pty(&["gc", "-n", "--keep-max-age", "0"]);
190+
expect_status(&dry_zero, 0);
191+
expect_contains(
192+
&dry_zero.stdout(),
193+
&format!("Would remove (keep expired after 0s): {name}"),
194+
);
195+
assert!(rig.meta_path(&name).exists());
196+
197+
// And the real pass then actually removes it.
198+
let real = rig.pty(&["gc"]);
199+
expect_status(&real, 0);
200+
expect_contains(
201+
&real.stdout(),
202+
&format!("Removed (keep expired after 7d): {name}"),
203+
);
204+
assert!(!rig.meta_path(&name).exists());
205+
}
206+
207+
/// node: tests/gc-keep-expiry.test.ts:239
208+
#[test]
209+
fn rejects_a_unit_less_non_zero_window() {
210+
let rig = Rig::new();
211+
let bare = rig.pty(&["gc", "--keep-max-age", "7"]);
212+
expect_failure(&bare);
213+
expect_contains(
214+
&bare.stderr(),
215+
"--keep-max-age expects a duration like 12h, 7d, or 0",
216+
);
217+
218+
let junk = rig.pty(&["gc", "--keep-max-age=soon"]);
219+
expect_failure(&junk);
220+
expect_contains(
221+
&junk.stderr(),
222+
"--keep-max-age expects a duration like 12h, 7d, or 0",
223+
);
224+
}

crates/pty-core/src/registry/mod.rs

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -63,9 +63,9 @@ pub use root::{
6363
metadata_path, pid_path, recovery_revision_path, root_length_check, session_dir, socket_path,
6464
};
6565
pub use tags::{
66-
EXACT_RESERVED_TAG_KEYS, GC_BOOKKEEPING_KEYS, KEEP_FALSEY, KEEP_TAG, extract_filter_tags,
67-
is_keep_requested, is_reserved_tag_key, matches_all_tags, reap_on_exit_default,
68-
should_reap_at_exit, strip_gc_bookkeeping,
66+
DEFAULT_KEEP_MAX_AGE_MS, EXACT_RESERVED_TAG_KEYS, GC_BOOKKEEPING_KEYS, KEEP_FALSEY, KEEP_TAG,
67+
extract_filter_tags, is_keep_expired, is_keep_requested, is_reserved_tag_key, matches_all_tags,
68+
reap_on_exit_default, should_reap_at_exit, strip_gc_bookkeeping,
6969
};
7070
pub use time::{
7171
iso8601, iso8601_from_epoch_ms, local_hms, now_epoch_ms, now_iso8601, parse_iso8601_ms,

crates/pty-core/src/registry/tags.rs

Lines changed: 47 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -2,9 +2,10 @@
22
//! `--filter-tag` matching, the `keep` tag, exit-time reap precedence, and
33
//! the gc bookkeeping keys a manual restart strips.
44
//!
5-
//! node: src/tags.ts; src/sessions.ts:1020-1097; src/cli.ts:4081-4100
5+
//! node: src/tags.ts; src/sessions.ts:1020-1109; src/cli.ts:4081-4100
66
7-
use super::metadata::TagMap;
7+
use super::metadata::{SessionMetadata, TagMap};
8+
use super::time::parse_iso8601_ms;
89

910
/// Keys pty itself treats as bookkeeping and hides from the default
1011
/// listing (`pty list --tags` shows them).
@@ -54,7 +55,9 @@ pub fn extract_filter_tags(args: &mut Vec<String>) -> Result<TagMap, String> {
5455
Ok(tags)
5556
}
5657

57-
/// Tag key that exempts a session from every form of dead-session reaping.
58+
/// Tag key that exempts a session from the daemon's exit-time self-reap
59+
/// unconditionally, and from `pty gc`'s sweep for a bounded window
60+
/// ([`DEFAULT_KEEP_MAX_AGE_MS`]).
5861
///
5962
/// node: src/sessions.ts:1020
6063
pub const KEEP_TAG: &str = "keep";
@@ -79,6 +82,47 @@ pub fn is_keep_requested(tags: Option<&TagMap>) -> bool {
7982
}
8083
}
8184

85+
/// How long `keep` holds a dead session against `pty gc`'s sweep, unless
86+
/// the operator overrides it with `pty gc --keep-max-age <dur>`. Seven days
87+
/// is long enough that "I killed it Friday, I'll look Monday" still works,
88+
/// and short enough that a fleet of agents tagging every session cannot
89+
/// grow the registry without bound.
90+
///
91+
/// node: src/sessions.ts:1084 (`DEFAULT_KEEP_MAX_AGE_MS`)
92+
pub const DEFAULT_KEEP_MAX_AGE_MS: i64 = 7 * 24 * 60 * 60 * 1000;
93+
94+
/// Has a dead `keep`-tagged session outlived its retention window?
95+
///
96+
/// Age is anchored on `exitedAt` when the daemon wrote an exit record, else
97+
/// `createdAt` (a `vanished` session never wrote one) — the same anchor
98+
/// precedence `pty list --older-than` uses. Metadata carrying neither, or an
99+
/// unparseable timestamp, has no age and therefore never expires: retaining
100+
/// an unaged record is the recoverable failure, deleting it is not.
101+
///
102+
/// `max_age_ms <= 0` expires everything, including unaged records — that is
103+
/// the explicit "sweep the keep backlog now" request, not an inference from
104+
/// a timestamp. Callers must apply this to dead sessions only; a running
105+
/// session is never a sweep candidate regardless of its age.
106+
///
107+
/// node: src/sessions.ts:1098-1109 (`isKeepExpired`)
108+
pub fn is_keep_expired(metadata: Option<&SessionMetadata>, now_ms: i64, max_age_ms: i64) -> bool {
109+
if max_age_ms <= 0 {
110+
return true;
111+
}
112+
let Some(meta) = metadata else {
113+
return false;
114+
};
115+
let anchor = meta
116+
.exited_at
117+
.as_deref()
118+
.filter(|s| !s.is_empty())
119+
.unwrap_or(meta.created_at.as_str());
120+
match parse_iso8601_ms(anchor) {
121+
Some(ts) => now_ms - ts >= max_age_ms,
122+
None => false,
123+
}
124+
}
125+
82126
/// The config default for exit-time reaping: `PTY_REAP_ON_EXIT` unset →
83127
/// reap; `false|0|no|off` → preserve; anything else → reap.
84128
///

0 commit comments

Comments
 (0)