diff --git a/CHANGELOG.md b/CHANGELOG.md index c2ce7fa..44598ce 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -69,6 +69,37 @@ An empty field means the process is gone or `ps` did not answer, and under load `ps` is the thing that goes quiet, so the kernel is asked again. +### Bounded `keep` retention in gc + +- **`keep=true` on a DEAD session now expires.** `pty gc`'s sweep skips a + `keep`-tagged exited/vanished session only while it has been dead for less + than `--keep-max-age` (default `7d`), then reclaims it. Exit-time retention + is unchanged and still unconditional; running sessions are never swept + whatever their age. Agents tag the session they are debugging right now and + never come back to untag it, so the previous forever-exemption grew the + registry without bound (740 of 911 sessions on one host). +- **New `pty gc --keep-max-age `** — `Ns`/`Nm`/`Nh`/`Nd`, or bare `0` to + sweep every dead `keep` session on this pass. A unit-less non-zero value is + rejected rather than guessed at. Age is anchored on `exitedAt`, falling back + to `createdAt` for a `vanished` session that never wrote one — the same + anchor precedence `pty list --older-than` uses. Records with neither + timestamp never expire (except under `0`). +- **`GcResult` gains `keepExpired: string[]`** — dead sessions swept despite a + `keep` tag, disjoint from `removed` so callers can report the two reasons + apart. Public `@compoundingtech/pty/client` API surface change; the module + also now exports `DEFAULT_KEEP_MAX_AGE_MS` and `isKeepExpired`. +- **CLI output** — `Removed (keep expired after 7d): ` (`Would remove …` + under `--dry-run`), a `N keep-expired sessions` term in the summary bar, and + the retained-session line now names the window it is counting down: + `Kept (keep tag): — swept once dead for 7d, or remove the keep tag to + reap it now`. +- Tests in `tests/gc-keep-expiry.test.ts` (8 new) cover: young keep session + retained, expired one swept and reported apart from the untagged sweep, + custom windows in both flag spellings, `0` sweeping a just-exited session, + `createdAt` anchoring for a session with no exit record, a running keep + session surviving `--keep-max-age 0`, non-mutating dry runs, and flag + validation. + ### Complete session termination - `pty kill` now stops the PTY child and its complete descendant tree. A diff --git a/README.md b/README.md index b463777..24a6e33 100644 --- a/README.md +++ b/README.md @@ -59,7 +59,7 @@ pty run -e -- npm test # ephemeral: reap even on `pt pty run --tag owner=forge -- node srv.js # tag a session with metadata pty run --env PORT=3000 --env MODE=dev -- node srv.js # persisted child env overlay pty run --unset-env NO_COLOR -- node srv.js # persisted inherited-env removal -pty run --tag keep=true -- npm test # keep it even past a gc sweep, until you rm it +pty run --tag keep=true -- npm test # keep it past a gc sweep for 7d after it dies pty run --cwd /path -- node server.js # run in a specific directory pty rename my-label # inside a session: add/change its displayName @@ -195,7 +195,7 @@ Two per-session flags override the configured default either way: | Flag | Effect | |---|---| -| tag `keep=true` | Force **preserve**, and survive even a `pty gc` sweep — metadata/`lastLines`/events last until you `pty rm` it. Wins over everything, including `--ephemeral`. | +| tag `keep=true` | Force **preserve**, including past a `pty gc` sweep — metadata/`lastLines`/events last until `pty gc --keep-max-age` (default 7d) after the session died, or until you `pty rm` it. Wins over everything, including `--ephemeral`. | | `pty run -e` (`--ephemeral`) | Force **reap**, on *any* shutdown incl. `pty kill` and `strategy=permanent`. `keep` still wins over it. | `strategy=permanent` sessions are always preserved (their supervisor reconciles @@ -220,15 +220,28 @@ the supporting daemon. ```sh pty run -d -- npm test # shipped default: reaped when it finishes PTY_REAP_ON_EXIT=false pty run -d -- npm test # preserved: peekable until gc sweeps it -pty run -d --tag keep=true -- npm test # force-keep, even past a gc sweep, until you rm it +pty run -d --tag keep=true -- npm test # force-keep, past a gc sweep, for 7d after it dies pty run -d -e -- npm test # ephemeral: reaps on any shutdown, leaves no trace pty rm mybuild # explicit removal beats keep ``` `pty gc`'s sweep reclaims preserved-and-finished (and `vanished`) non-permanent sessions — see [Auto-running gc](#auto-running-gc). `pty list` is strictly -observational and never removes registry state. `keep=true` and -`strategy=permanent` are exempt from the gc sweep. +observational and never removes registry state. `strategy=permanent` sessions +are exempt from the sweep; `keep=true` is exempt for a bounded window. + +**`keep` expires.** A `keep`-tagged session is exempt from the sweep until it +has been *dead* longer than `pty gc --keep-max-age ` (default `7d`), then +it is swept like any other stale record and reported as +`Removed (keep expired after 7d): `. Nobody ever comes back to untag a +session they pinned mid-debug, so an unbounded exemption turns the registry +into an append-only log. Running sessions are never swept, whatever their age. + +```sh +pty gc --keep-max-age 30d # a month of retention instead of a week +pty gc --keep-max-age 0 # the keep exemption is over: sweep the backlog now +pty gc -n --keep-max-age 0 # …preview that first; keep-expired sessions are counted separately +``` ### Events @@ -387,7 +400,7 @@ Cycles (A→B, B→A) resolve deterministically by name-sorted iteration: whiche `pty gc` is a one-shot reconciliation pass. The intended deployment is to run it on a short interval so permanent sessions come back quickly and orphans get cleaned promptly. The CLI ships an install helper for macOS: -Whether finished sessions need the sweep depends on [`PTY_REAP_ON_EXIT`](#session-lifecycle-and-cleanup): under the shipped `reap` default they self-clean at exit, so the sweep's finished-session duty is mostly `vanished` sessions (daemon killed outright, so it never ran its own cleanup) plus anything left listed by `preserve` mode. `pty list` only observes this state; it never cleans it up. So the interval primarily buys you respawn latency for permanents and orphan-kill promptness — and, in `preserve` mode, `pty ls` hygiene. `keep=true` and `strategy=permanent` sessions are exempt. +Whether finished sessions need the sweep depends on [`PTY_REAP_ON_EXIT`](#session-lifecycle-and-cleanup): under the shipped `reap` default they self-clean at exit, so the sweep's finished-session duty is mostly `vanished` sessions (daemon killed outright, so it never ran its own cleanup) plus anything left listed by `preserve` mode. `pty list` only observes this state; it never cleans it up. So the interval primarily buys you respawn latency for permanents and orphan-kill promptness — and, in `preserve` mode, `pty ls` hygiene. `strategy=permanent` sessions are exempt; `keep=true` sessions are exempt until they have been dead longer than `--keep-max-age` (default 7d), which an interval-driven gc then reclaims on its own. ```sh pty gc --print-launchd-plist > ~/Library/LaunchAgents/com.compoundingtech.pty.gc.plist 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/docs/disk-layout.md b/docs/disk-layout.md index 3ca5a16..6f2ef5a 100644 --- a/docs/disk-layout.md +++ b/docs/disk-layout.md @@ -110,8 +110,8 @@ the historical ambient-inheritance behavior. - `strategy.abandon-if-cwd-gone=false` — opts a permanent session OUT of the on-by-default cwd-gone reap in `pty gc` step 1.5. Only meaningful with `strategy=permanent`. - `strategy.idle-days=` — opts a permanent session INTO idle-reap: `pty gc` reaps it when `lastAttachAt` is older than N days. Takes precedence over the global `--idle-days` flag. - `parent=` — `pty gc` orphan-kills this session (SIGTERM + cleanup) when the referenced session's daemon is no longer alive. Combinator with `strategy=permanent` is well-defined: orphan-kill wins. - - `keep=true` — exempts the session from reaping, both the daemon's exit-time self-cleanup and `pty gc`'s sweep. Its metadata, `lastLines`, and events file survive its death until an explicit `pty rm`. Any value other than `false`/`0`/`no`/`off` counts as set, so a mis-spelled value errs toward retaining. Without this tag, a non-permanent session's files are gone the moment its command finishes. -- Lifetime: a non-permanent session's files are removed by its own daemon during shutdown once the child process terminates. Files therefore outlive the process only for `keep`, `strategy=permanent`, external `pty kill`, and `vanished` sessions (SIGKILLed daemon — no cleanup code ran). Readers that poll these files after a session finishes must set `keep=true` or accept the race. + - `keep=true` — exempts the session from reaping: unconditionally from the daemon's exit-time self-cleanup, and from `pty gc`'s sweep until the session has been dead longer than `pty gc --keep-max-age ` (default 7d; `0` sweeps every dead keep session on that pass). Age is anchored on `exitedAt`, or `createdAt` for a `vanished` session that never wrote one. Its metadata, `lastLines`, and events file therefore survive its death until that window elapses or an explicit `pty rm`, whichever comes first; a running session is never swept regardless of age. Any value other than `false`/`0`/`no`/`off` counts as set, so a mis-spelled value errs toward retaining. Without this tag, a non-permanent session's files are gone the moment its command finishes. +- Lifetime: a non-permanent session's files are removed by its own daemon during shutdown once the child process terminates. Files therefore outlive the process only for `keep` (bounded by `--keep-max-age`), `strategy=permanent`, external `pty kill`, and `vanished` sessions (SIGKILLed daemon — no cleanup code ran). Readers that poll these files after a session finishes must set `keep=true` or accept the race. - Concurrent writers: last-write-wins; readers never see torn files. Cross-process writers can lose updates to the read-modify-write window. ## `.events.jsonl` (tier 1) diff --git a/src/cli.ts b/src/cli.ts index bd87609..cb67e06 100644 --- a/src/cli.ts +++ b/src/cli.ts @@ -45,6 +45,7 @@ import { getMetadataPath, DEFAULT_SESSION_DIR, hasProcessExitedForReap, + DEFAULT_KEEP_MAX_AGE_MS, type SessionInfo, type SessionMetadata, } from "./sessions.ts"; @@ -334,7 +335,7 @@ Won't remove a running session — kill it first. Examples: pty rm myserver`, - gc: `Usage: pty gc [-n] [--idle-days N] [--fast-fail-window=N] [--fast-fail-limit=N] + gc: `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, @@ -342,11 +343,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 @@ -354,6 +358,7 @@ Flags: Examples: pty gc --dry-run + pty gc --keep-max-age 0 pty gc --print-launchd-plist > ~/Library/LaunchAgents/com.compoundingtech.pty.gc.plist`, tag: `Usage: pty tag Show tags @@ -583,6 +588,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 @@ -1443,6 +1450,7 @@ async function main(): Promise { let idleDays: number | undefined; let fastFailWindowSec: number | undefined; let fastFailLimit: number | undefined; + let keepMaxAgeMs: number | undefined; const parsePositive = (flag: string, raw: string): number => { const v = parseInt(raw, 10); if (!Number.isFinite(v) || v <= 0) { @@ -1451,6 +1459,19 @@ async function main(): Promise { } return v; }; + // 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. + const parseAge = (flag: string, raw: string): number => { + if (raw.trim() === "0") return 0; + const ms = parseDuration(raw); + if (ms == null) { + console.error(`pty gc: ${flag} expects a duration like 12h, 7d, or 0 (got "${raw}")`); + process.exit(1); + } + return ms; + }; for (let i = 0; i < gcArgs.length; i++) { const a = gcArgs[i]; if (a === "--interval" && i + 1 < gcArgs.length) { @@ -1469,13 +1490,17 @@ async function main(): Promise { fastFailLimit = parsePositive("--fast-fail-limit", gcArgs[++i]); } else if (a.startsWith("--fast-fail-limit=")) { fastFailLimit = parsePositive("--fast-fail-limit", a.slice("--fast-fail-limit=".length)); + } else if (a === "--keep-max-age" && i + 1 < gcArgs.length) { + keepMaxAgeMs = parseAge("--keep-max-age", gcArgs[++i]); + } else if (a.startsWith("--keep-max-age=")) { + keepMaxAgeMs = parseAge("--keep-max-age", a.slice("--keep-max-age=".length)); } } if (printPlist) { printLaunchdPlist(interval); break; } - await cmdGc(dryRun, idleDays, fastFailWindowSec, fastFailLimit); + await cmdGc({ dryRun, idleDays, fastFailWindowSec, fastFailLimit, keepMaxAgeMs }); break; } @@ -3157,13 +3182,16 @@ async function cmdRm(name: string): Promise { console.log(`Session "${name}" removed.`); } -async function cmdGc( - dryRun: boolean, - idleDays?: number, - fastFailWindowSec?: number, - fastFailLimit?: number, -): Promise { - const result = await gc({ dryRun, idleDays, fastFailWindowSec, fastFailLimit }); +async function cmdGc(opts: { + dryRun: boolean; + idleDays?: number; + fastFailWindowSec?: number; + fastFailLimit?: number; + keepMaxAgeMs?: number; +}): Promise { + const { dryRun } = opts; + const result = await gc(opts); + const keepMaxAgeMs = opts.keepMaxAgeMs ?? DEFAULT_KEEP_MAX_AGE_MS; const prunedTags = await pruneOrphanLayoutTags({ dryRun }); const killedVerb = dryRun ? "Would kill orphan child" : "Killed orphan child"; @@ -3208,11 +3236,21 @@ async function cmdGc( for (const name of result.removed) { console.log(`${removeVerb}: ${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. + for (const name of result.keepExpired) { + console.log( + `${removeVerb} (keep expired after ${formatDuration(keepMaxAgeMs)}): ${name}`, + ); + } // Deliberately NOT counted as an action below: a kept session is a // no-op. It is printed anyway so "why is this dead session still // listed?" has a visible answer instead of looking like a gc bug. for (const name of result.kept) { - console.log(`Kept (keep tag): ${name} — remove the keep tag to reap it`); + console.log( + `Kept (keep tag): ${name} — swept once dead for ${formatDuration(keepMaxAgeMs)}, or remove the keep tag to reap it now`, + ); } for (const { name, removedKeys } of prunedTags) { console.log( @@ -3230,6 +3268,7 @@ async function cmdGc( result.flapped.length + result.flappingSkipped.length + result.removed.length + + result.keepExpired.length + totalTags; if (totalActions === 0) { @@ -3262,6 +3301,9 @@ async function cmdGc( if (result.removed.length > 0) { parts.push(`${result.removed.length} stale session${result.removed.length === 1 ? "" : "s"}`); } + if (result.keepExpired.length > 0) { + parts.push(`${result.keepExpired.length} keep-expired session${result.keepExpired.length === 1 ? "" : "s"}`); + } if (totalTags > 0) { parts.push(`${totalTags} orphan tag${totalTags === 1 ? "" : "s"}`); } diff --git a/src/client-api.ts b/src/client-api.ts index 680f32a..87982ba 100644 --- a/src/client-api.ts +++ b/src/client-api.ts @@ -10,8 +10,10 @@ export { cleanupSocket, cleanupAll, // Exposed for the same reason as `isReservedTagKey`: downstream tools // (relay, layout, supervisors) need to answer "is this session exempt - // from reaping?" without re-deriving which tag values count as set. + // from reaping?" without re-deriving which tag values count as set — + // including how long the `keep` exemption lasts against `pty gc`. KEEP_TAG, isKeepRequested, shouldReapAtExit, + DEFAULT_KEEP_MAX_AGE_MS, isKeepExpired, type SessionInfo, type SessionMetadata, type MetadataPatch, type MetadataPatchResult, type SessionExitEvidence, type SessionExitEvidenceTail, type SessionExitEvidenceResult, type RemoveSessionGenerationResult, diff --git a/src/completions.ts b/src/completions.ts index 81bb919..3bbb586 100644 --- a/src/completions.ts +++ b/src/completions.ts @@ -203,6 +203,7 @@ const COMMANDS: readonly CommandSpec[] = [ flags: [ { name: "dry-run", short: "n", desc: "Preview without changing anything" }, { name: "idle-days", desc: "Reap permanents with no attach in N days" }, + { name: "keep-max-age", desc: "Keep-tag retention for dead sessions (default 7d; 0 = now)" }, { name: "fast-fail-window", desc: "Fast-fail window (seconds; default 60)" }, { name: "fast-fail-limit", desc: "Consecutive fast fails before flapping (default 3)" }, { name: "print-launchd-plist", desc: "Emit a launchd plist that runs pty gc" }, diff --git a/src/sessions.ts b/src/sessions.ts index 8da082d..4b794e3 100644 --- a/src/sessions.ts +++ b/src/sessions.ts @@ -1038,11 +1038,18 @@ export async function listSessions(options: ListSessionsOptions = {}): Promise (a.name < b.name ? -1 : a.name > b.name ? 1 : 0)); } -/** Tag key that exempts a session from every form of dead-session reaping: - * the exit-time self-reap in the daemon AND `pty gc`'s sweep of exited +/** Tag key that exempts a session from the exit-time self-reap in the daemon + * AND — for a bounded retention window — from `pty gc`'s sweep of exited * non-permanent sessions. Set it when you want a session's metadata, * `lastLines`, and events file to survive its own death so you can inspect - * them afterwards. Mirrors the `keep` field in the agent spec. */ + * them afterwards. Mirrors the `keep` field in the agent spec. + * + * The gc exemption is time-boxed (`DEFAULT_KEEP_MAX_AGE_MS`), not eternal: + * agents set `keep=true` to protect a session they are debugging *now*, and + * nobody comes back to untag it, so an unbounded exemption turns the + * registry into an append-only log. Exit-time retention is unconditional — + * expiry only ever happens on a gc pass, and only once the session has been + * dead longer than the window. */ export const KEEP_TAG = "keep"; /** Values that read as "no" for the `keep` tag; everything else reads as @@ -1069,6 +1076,38 @@ export function isKeepRequested(tags?: Record): boolean { return !KEEP_FALSEY.has(raw.trim().toLowerCase()); } +/** 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. */ +export const DEFAULT_KEEP_MAX_AGE_MS = 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 session is the recoverable failure, deleting it is not. + * + * `maxAgeMs <= 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. */ +export function isKeepExpired( + metadata: SessionMetadata | null | undefined, + nowMs: number, + maxAgeMs: number, +): boolean { + if (maxAgeMs <= 0) return true; + const anchor = metadata?.exitedAt ?? metadata?.createdAt; + if (!anchor) return false; + const ts = Date.parse(anchor); + if (!Number.isFinite(ts)) return false; + return nowMs - ts >= maxAgeMs; +} + /** Should the daemon remove its own registry entry as it shuts down? * * Exit-time reaping is CONFIGURABLE. `defaultReap` is the config default (see @@ -1403,9 +1442,16 @@ export interface GcResult { * list as the preview. */ removed: string[]; /** Dead non-permanent sessions left in place because they carry the - * `keep` tag. Reported rather than silently skipped so an operator can - * see why `pty ls` still shows a dead session after a gc pass. */ + * `keep` tag and are still inside its retention window. Reported rather + * than silently skipped so an operator can see why `pty ls` still shows + * a dead session after a gc pass. */ kept: string[]; + /** Dead non-permanent sessions swept DESPITE a `keep` tag because they + * outlived the retention window (`opts.keepMaxAgeMs`). Disjoint from + * `removed`, which holds the untagged sweep, so a caller can report + * "the keep tag expired" separately from ordinary stale-session + * cleanup. Under `dryRun: true` this is the preview. */ + keepExpired: string[]; /** Children killed because their `parent=` referent is dead or missing. */ killedOrphanChildren: { name: string; parent: string; reason: "missing" | "dead" }[]; /** Live `strategy=permanent` sessions reaped because they've been @@ -1523,7 +1569,10 @@ function commandFingerprint(command: string, args: string[]): string { * skip it on subsequent ticks. Auto-reset when the stored command * changes; manual reset via `pty tag --rm strategy.status`. * 3. Residual sweep: exited/vanished sessions that aren't permanent - * and aren't tagged `keep` get `cleanupAll`'d. + * get `cleanupAll`'d. A `keep`-tagged session is exempt only until + * it has been dead longer than `opts.keepMaxAgeMs` (default + * `DEFAULT_KEEP_MAX_AGE_MS`), after which it is swept and reported + * under `keepExpired` instead of `removed`. * * Step 3 is now a BACKSTOP rather than the primary path: a non-permanent * session that runs to completion reaps itself as it shuts down (see @@ -1534,8 +1583,8 @@ function commandFingerprint(command: string, args: string[]): string { * easy to miss: the exit path deliberately retains a session stopped * from outside, but the child's `onExit` still wrote an exit record, * so the session lands here as `status=exited` and gets swept. The - * retention is until the next sweep, not forever — `keep` is what - * makes it forever. + * retention is until the next sweep, not forever — `keep` extends it + * to the retention window. * - `status=vanished` sessions — the daemon was SIGKILL'd / OOM-killed * / lost to a reboot, so no exit-time code ran at all. This is the * case exit-time cleanup structurally *cannot* cover, since the @@ -1550,12 +1599,17 @@ export async function gc( idleDays?: number; fastFailWindowSec?: number; fastFailLimit?: number; + /** Retention window for `keep`-tagged dead sessions. Defaults to + * `DEFAULT_KEEP_MAX_AGE_MS`; `0` sweeps every dead `keep` session on + * this pass. Never applies to running sessions. */ + keepMaxAgeMs?: number; } = {}, ): Promise { const dryRun = !!opts.dryRun; const globalIdleDays = opts.idleDays; const globalFastFailWindow = opts.fastFailWindowSec; const globalFastFailLimit = opts.fastFailLimit; + const keepMaxAgeMs = opts.keepMaxAgeMs ?? DEFAULT_KEEP_MAX_AGE_MS; const rawCandidates = await inventoryRawCleanupCandidates(); const rawRemoved: string[] = []; if (dryRun) { @@ -1716,8 +1770,10 @@ export async function gc( // if it failed we leave the metadata around so the next tick can try // again. const finalList = dryRun ? initial : await listSessions(); + const nowMs = Date.now(); const removed: string[] = [...rawRemoved]; const kept: string[] = []; + const keepExpired: string[] = []; for (const s of finalList) { if (!isGone(s.status)) continue; if (s.metadata?.tags?.strategy === "permanent") continue; @@ -1725,20 +1781,26 @@ export async function gc( // exit-time cleanup already honours it; if gc did not, a `keep` // session would merely survive its own exit only to be swept moments // later by the next tick — which is not "keep" in any useful sense. - if (isKeepRequested(s.metadata?.tags)) { + // The exemption is bounded, though: once the session has been dead + // longer than the retention window it is swept like any other stale + // record, just reported under its own bucket so the reason is visible. + const keepRequested = isKeepRequested(s.metadata?.tags); + if (keepRequested && !isKeepExpired(s.metadata, nowMs, keepMaxAgeMs)) { kept.push(s.name); continue; } + const bucket = keepRequested ? keepExpired : removed; if (dryRun) { - removed.push(s.name); + bucket.push(s.name); } else if (await cleanupObservedSession(s)) { - removed.push(s.name); + bucket.push(s.name); } } return { removed, kept, + keepExpired, killedOrphanChildren, abandoned, reapSkipped, diff --git a/tests/gc-keep-expiry.test.ts b/tests/gc-keep-expiry.test.ts new file mode 100644 index 0000000..d592565 --- /dev/null +++ b/tests/gc-keep-expiry.test.ts @@ -0,0 +1,249 @@ +// `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 (740 of 911 sessions on one host). +// +// 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. +// +// Sessions are fabricated on disk rather than spawned: the policy is a +// comparison against `exitedAt`/`createdAt`, so writing the record directly is +// both exact about age and free of daemon-startup waits. + +import { describe, it, expect, afterEach, afterAll } from "vitest"; +import * as fs from "node:fs"; +import * as os from "node:os"; +import * as path from "node:path"; +import { fileURLToPath } from "node:url"; +import { spawnSync } from "node:child_process"; + +const __dirname = path.dirname(fileURLToPath(import.meta.url)); +const nodeBin = process.execPath; +const cliPath = path.join(__dirname, "..", "dist", "cli.js"); + +const testRoot = fs.mkdtempSync(path.join(os.tmpdir(), "pty-gc-keep-")); +afterAll(() => { + fs.rmSync(testRoot, { recursive: true, force: true, maxRetries: 3, retryDelay: 100 }); +}); + +let sessionDirs: string[] = []; + +function makeSessionDir(): string { + const dir = fs.mkdtempSync(path.join(testRoot, "d-")); + sessionDirs.push(dir); + return dir; +} + +let nameCounter = 0; +function uniqueName(): string { + return `keep${++nameCounter}-${Math.random().toString(36).slice(2, 6)}`; +} + +function runCli(sessionDir: string, ...args: string[]) { + return spawnSync(nodeBin, [cliPath, ...args], { + env: { ...process.env, PTY_SESSION_DIR: sessionDir }, + encoding: "utf-8", + timeout: 10000, + }); +} + +/** Metadata of a cleanly-exited session, dead for `deadForMs`. Tagged `keep` + * unless `tags` says otherwise. */ +function writeExitedKeep( + sessionDir: string, + name: string, + deadForMs: number, + tags: Record = { keep: "true" }, +): string { + const metaPath = path.join(sessionDir, `${name}.json`); + const exitedAt = new Date(Date.now() - deadForMs).toISOString(); + fs.writeFileSync(metaPath, JSON.stringify({ + command: "cat", + args: [], + displayCommand: "cat", + cwd: os.tmpdir(), + createdAt: exitedAt, + exitedAt, + exitCode: 0, + tags, + })); + return metaPath; +} + +const DAY_MS = 24 * 60 * 60 * 1000; + +interface ListedSession { + name: string; + status: string; +} + +function listSessionStatus(sessionDir: string, name: string): string | undefined { + const r = runCli(sessionDir, "list", "--json"); + expect(r.status, r.stderr).toBe(0); + const parsed: unknown = JSON.parse(r.stdout); + if (!Array.isArray(parsed)) return undefined; + const sessions = parsed as ListedSession[]; + return sessions.find((s) => s.name === name)?.status; +} + +afterEach(() => { + for (const dir of sessionDirs) { + try { + for (const e of fs.readdirSync(dir)) { try { fs.unlinkSync(path.join(dir, e)); } catch {} } + } catch {} + } + sessionDirs = []; +}); + +describe("pty gc — keep-tag expiry", () => { + it("keeps an exited keep session that is younger than the default window", () => { + const dir = makeSessionDir(); + const name = uniqueName(); + const metaPath = writeExitedKeep(dir, name, 2 * DAY_MS); + + const r = runCli(dir, "gc"); + expect(r.status, r.stderr).toBe(0); + expect(r.stdout).toContain(`Kept (keep tag): ${name}`); + expect(r.stdout).not.toContain("keep expired"); + expect(fs.existsSync(metaPath)).toBe(true); + }, 10000); + + it("sweeps an exited keep session older than the default window, reported apart from the plain sweep", () => { + const dir = makeSessionDir(); + const expired = uniqueName(); + const stale = uniqueName(); + const expiredPath = writeExitedKeep(dir, expired, 30 * DAY_MS); + // A same-age session WITHOUT the tag: proves the two buckets stay + // distinct rather than one absorbing the other. + const stalePath = writeExitedKeep(dir, stale, 30 * DAY_MS, {}); + + const r = runCli(dir, "gc"); + expect(r.status, r.stderr).toBe(0); + expect(r.stdout).toContain(`Removed (keep expired after 7d): ${expired}`); + expect(r.stdout).toContain(`Removed: ${stale}`); + expect(r.stdout).toContain("1 stale session"); + expect(r.stdout).toContain("1 keep-expired session"); + expect(fs.existsSync(expiredPath)).toBe(false); + expect(fs.existsSync(stalePath)).toBe(false); + }, 10000); + + it("honours a custom --keep-max-age window in both flag spellings", () => { + const dir = makeSessionDir(); + const spaced = uniqueName(); + const equals = uniqueName(); + const spacedPath = writeExitedKeep(dir, spaced, 2 * 60 * 60 * 1000); + const equalsPath = writeExitedKeep(dir, equals, 2 * 60 * 60 * 1000); + + // 3h window: both sessions are 2h dead, so both survive. + const kept = runCli(dir, "gc", "--keep-max-age", "3h"); + expect(kept.status, kept.stderr).toBe(0); + expect(kept.stdout).toContain(`Kept (keep tag): ${spaced}`); + expect(kept.stdout).toContain(`Kept (keep tag): ${equals}`); + expect(fs.existsSync(spacedPath)).toBe(true); + + // 1h window: both are past it. + const swept = runCli(dir, "gc", "--keep-max-age=1h"); + expect(swept.status, swept.stderr).toBe(0); + expect(swept.stdout).toContain(`Removed (keep expired after 1h): ${spaced}`); + expect(swept.stdout).toContain(`Removed (keep expired after 1h): ${equals}`); + expect(fs.existsSync(spacedPath)).toBe(false); + expect(fs.existsSync(equalsPath)).toBe(false); + }, 10000); + + it("--keep-max-age 0 sweeps a keep session that just exited", () => { + const dir = makeSessionDir(); + const name = uniqueName(); + const metaPath = writeExitedKeep(dir, name, 0); + + const r = runCli(dir, "gc", "--keep-max-age", "0"); + expect(r.status, r.stderr).toBe(0); + expect(r.stdout).toContain(`Removed (keep expired after 0s): ${name}`); + expect(fs.existsSync(metaPath)).toBe(false); + }, 10000); + + it("sweeps an expired keep session with no exit record, anchored on createdAt", () => { + const dir = makeSessionDir(); + const name = uniqueName(); + // A vanished session (SIGKILLed daemon) never wrote `exitedAt`, so its + // age comes from `createdAt` — the same anchor precedence `pty list + // --older-than` uses. + const metaPath = path.join(dir, `${name}.json`); + fs.writeFileSync(metaPath, JSON.stringify({ + command: "cat", + args: [], + displayCommand: "cat", + cwd: os.tmpdir(), + createdAt: new Date(Date.now() - 30 * DAY_MS).toISOString(), + tags: { keep: "true" }, + })); + + const r = runCli(dir, "gc"); + expect(r.status, r.stderr).toBe(0); + expect(r.stdout).toContain(`Removed (keep expired after 7d): ${name}`); + expect(fs.existsSync(metaPath)).toBe(false); + }, 10000); + + it("never sweeps a RUNNING keep session, even at --keep-max-age 0", () => { + const dir = makeSessionDir(); + const name = uniqueName(); + const metaPath = path.join(dir, `${name}.json`); + // The test runner's own pid stands in for a live daemon (same device as + // tests/list-filters.test.ts): 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. + fs.writeFileSync(path.join(dir, `${name}.pid`), String(process.pid)); + fs.writeFileSync(metaPath, JSON.stringify({ + command: "cat", + args: [], + displayCommand: "cat", + cwd: os.tmpdir(), + createdAt: new Date(Date.now() - 30 * DAY_MS).toISOString(), + tags: { keep: "true" }, + })); + expect(listSessionStatus(dir, name)).toBe("running"); + + const r = runCli(dir, "gc", "--keep-max-age", "0"); + expect(r.status, r.stderr).toBe(0); + expect(r.stdout).not.toContain(name); + expect(fs.existsSync(metaPath)).toBe(true); + expect(listSessionStatus(dir, name)).toBe("running"); + }, 10000); + + it("--dry-run previews keep expiry without removing anything", () => { + const dir = makeSessionDir(); + const name = uniqueName(); + const metaPath = writeExitedKeep(dir, name, 30 * DAY_MS); + + const dry = runCli(dir, "gc", "--dry-run"); + expect(dry.status, dry.stderr).toBe(0); + expect(dry.stdout).toContain(`Would remove (keep expired after 7d): ${name}`); + expect(dry.stdout).toContain("1 keep-expired session"); + expect(dry.stdout).toContain("Dry run"); + expect(fs.existsSync(metaPath)).toBe(true); + + // Zero-window dry run is equally non-mutating. + const dryZero = runCli(dir, "gc", "-n", "--keep-max-age", "0"); + expect(dryZero.status, dryZero.stderr).toBe(0); + expect(dryZero.stdout).toContain(`Would remove (keep expired after 0s): ${name}`); + expect(fs.existsSync(metaPath)).toBe(true); + + // And the real pass then actually removes it. + const real = runCli(dir, "gc"); + expect(real.status, real.stderr).toBe(0); + expect(real.stdout).toContain(`Removed (keep expired after 7d): ${name}`); + expect(fs.existsSync(metaPath)).toBe(false); + }, 15000); + + it("rejects a unit-less non-zero --keep-max-age", () => { + const dir = makeSessionDir(); + const bare = runCli(dir, "gc", "--keep-max-age", "7"); + expect(bare.status).not.toBe(0); + expect(bare.stderr).toContain("--keep-max-age expects a duration like 12h, 7d, or 0"); + + const junk = runCli(dir, "gc", "--keep-max-age=soon"); + expect(junk.status).not.toBe(0); + expect(junk.stderr).toContain("--keep-max-age expects a duration like 12h, 7d, or 0"); + }, 10000); +});