follow-ups: restart/up clear flap mark, [flapping] badge, PTY_ROOT length backstop (#57) · compounding.tech/pty@c196230

+7

CHANGELOG.md

··· 2 2 3 3 ## Unreleased 4 4 5 + ### Follow-ups to #56 (fast-fail cap) + PTY_ROOT length backstop 6 + 7 + - **`pty restart` and `pty up` now clear the `pty gc` flapping bookkeeping.** Manual restart is an operator "please try again" signal — dropping `strategy.status`, `strategy.consecutive-fast-fails`, `strategy.last-respawn-at`, and `strategy.command-hash` on the restarted session gives it a clean slate, so the next `pty gc` tick isn't a no-op (silent skip against a stale flapping flag). Auto-reset on command-hash divergence already handled the toml-edit case; this handles the operator-intervenes-without-edit case that gc otherwise can't infer. Applies to both `pty restart` and `pty up`'s "already running, tag-sync" path. 8 + - **`pty list` renders `[flapping]`** in place of `[permanent]` when a session carries `strategy.status=flapping`. Red instead of yellow — the flag stands out from ordinary permanent sessions because the operator's expectation has changed (`pty gc` has stopped respawning it on purpose). 9 + - **Startup-time PTY_ROOT length backstop.** When the resolved root's byte length + a default-shape `/<8-char-id>.sock` suffix (14 bytes) would exceed the `sockaddr_un.sun_path` 104-byte kernel limit, `pty` errors before any subcommand runs. The error names the root and points the finger at the root, not the session name — the previous fallthrough error at spawn time read as if a random-id name were the problem. Backstop respects `--root <shorter>` overrides. 10 + - Tests in `tests/gc-flap-clear-badge-root-len.test.ts` (7 new) cover: restart-clears-bookkeeping, list shows [flapping] and hides [permanent] when both would apply, list still shows [permanent] otherwise, backstop errors on `pty list` with too-deep root, backstop fires before subcommand parsing, root at the usable threshold succeeds, `--root <shorter>` overrides a too-long env. 11 + 5 12 ### `pty gc` fast-fail respawn cap (fixes #54) 6 13 7 14 - **New:** `pty gc` STEP-2 now detects a crash-looping permanent session and stops respawning it before it churns forever. A respawn whose leaf exits within `strategy.fast-fail-window` seconds (default 60) of the previous respawn counts as a fast fail; after `strategy.fast-fail-limit` consecutive fast fails (default 3) the session gets `strategy.status=flapping` written to its tags and a `session_flapping` event, and subsequent gc ticks silently skip it. This is the crash-loop-with-live-cwd case that the earlier `cwd-gone`/`idle` reap (#47, #50) didn't cover.

+5 -2

README.md

··· 268 268 269 269 **Fast-fail cap** — a permanent session whose leaf exits within `strategy.fast-fail-window` seconds of its previous `pty gc` respawn counts as a fast fail. After `strategy.fast-fail-limit` consecutive fast fails, `pty gc` writes `strategy.status=flapping` on the session, emits a `session_flapping` event, and stops respawning it. Subsequent gc ticks print `Skipped (flapping): <name>` and take no action. Defaults: 60 s window, 3 consecutive fast fails. 270 270 271 + A flagged session shows `[flapping]` (red) in `pty list` in place of `[permanent]` — the operator's expectation has changed, so the badge reflects that. 272 + 271 273 Reset a flagged session with one of: 272 274 273 - - `pty tag <name> --rm strategy.status` — clear the mark, `pty gc` retries on the next tick. 274 - - Edit the session's `pty.toml` command — the classifier notices the SHA-256 fingerprint change and auto-resets the counter and mark. 275 + - `pty restart <name>` or `pty up` — the manual respawn drops all fast-fail bookkeeping (`strategy.status`, `strategy.consecutive-fast-fails`, `strategy.last-respawn-at`, `strategy.command-hash`), treating restart as an operator "please try again" signal. 276 + - `pty tag <name> --rm strategy.status` — surgical reset that clears only the mark, leaving the counter intact for observability. 277 + - Edit the session's `pty.toml` command — the classifier notices the SHA-256 fingerprint change and auto-resets the counter and mark on the next gc tick. 275 278 276 279 Per-session overrides tune the cap without editing gc's globals: 277 280

+80 -1

src/cli.ts

··· 224 224 args.splice(rootIdx, 2); 225 225 } 226 226 227 + // Fail-loud backstop for the sockaddr_un.sun_path 104-byte kernel limit. 228 + // `validateName()` already catches a too-long root at spawn time by 229 + // computing the full socket path, but its error message reads as if 230 + // the name were the problem, and it fires per-invocation only when a 231 + // spawn happens. This check catches the pathological deep-PTY_ROOT 232 + // case at startup — before any subcommand runs — and points the 233 + // finger at the root, not the name. 234 + // 235 + // Threshold: an 8-char random session id (the default `pty run` 236 + // shape) produces a socket suffix of `/xxxxxxxx.sock` = 14 bytes. 237 + // A root whose length + 14 exceeds 104 can't host a default-id 238 + // session and is unusable. Callers who intentionally want tiny 239 + // 1-char names on a nearly-full root can side-step by shortening 240 + // the root; there's no correct behavior for a genuinely-too-long 241 + // root, so we fail rather than limp. 242 + const resolvedRoot = process.env.PTY_ROOT ?? process.env.PTY_SESSION_DIR; 243 + if (resolvedRoot && resolvedRoot.length > 0) { 244 + const SUN_PATH_MAX = 104; 245 + const SOCK_SUFFIX_BYTES = "/".length + 8 + ".sock".length; 246 + const rootBytes = Buffer.byteLength(resolvedRoot, "utf-8"); 247 + if (rootBytes + SOCK_SUFFIX_BYTES > SUN_PATH_MAX) { 248 + const usable = SUN_PATH_MAX - SOCK_SUFFIX_BYTES; 249 + console.error( 250 + `pty: PTY_ROOT is too long — ${rootBytes} bytes; must be ≤ ${usable} bytes for the socket path to fit the ${SUN_PATH_MAX}-byte kernel limit.\n` + 251 + ` root: ${resolvedRoot}\n` + 252 + ` Shorten the root (or use \`pty --root <shorter-path>\` for a one-off).` 253 + ); 254 + process.exit(1); 255 + } 256 + } 257 + 227 258 // Interactive-mode flags (--preselect-new, --filter-tag) can appear before 228 259 // the subcommand. Peek at the subcommand without consuming flags; if it's 229 260 // the interactive TUI (none, "i", or "interactive"), consume those flags ··· 2637 2668 const newKeySet = new Set(userTomlKeys); 2638 2669 const removals = prevTomlKeys.filter((k) => !newKeySet.has(k)); 2639 2670 2671 + // Manual `pty up` is an operator "reset" signal, same shape as 2672 + // `pty restart` in cmdRestart above. Drop any `pty gc` flapping 2673 + // bookkeeping the session may have accumulated so a re-`pty up` 2674 + // gives the session a clean slate. These keys are gc-owned, never 2675 + // toml-declared, so they aren't in `prevTomlKeys`. 2676 + for (const k of [ 2677 + "strategy.status", 2678 + "strategy.consecutive-fast-fails", 2679 + "strategy.last-respawn-at", 2680 + "strategy.command-hash", 2681 + ]) { 2682 + if (currentTags[k] !== undefined && !removals.includes(k)) removals.push(k); 2683 + } 2684 + 2640 2685 const label = bound.metadata?.displayName ?? bound.name; 2641 2686 if (Object.keys(updates).length > 0 || removals.length > 0) { 2642 2687 try { ··· 2831 2876 } 2832 2877 2833 2878 cleanupAll(name); 2834 - await spawnDaemon({ name, command: meta.command, args: meta.args, displayCommand: meta.displayCommand, cwd: meta.cwd, tags: meta.tags }); 2879 + // Manual restart is an operator "please try again" signal — drop any 2880 + // `strategy.status=flapping` mark and its bookkeeping so the fresh 2881 + // spawn isn't skipped by `pty gc` on the next tick. Auto-reset on 2882 + // command edit already clears these; this handles the operator- 2883 + // intervenes-without-edit case that gc otherwise can't infer. 2884 + const restartTags = clearFlappingBookkeeping(meta.tags); 2885 + await spawnDaemon({ name, command: meta.command, args: meta.args, displayCommand: meta.displayCommand, cwd: meta.cwd, tags: restartTags }); 2835 2886 console.log(`Session "${name}" restarted.`); 2836 2887 2837 2888 // Nesting guard: restart itself is fine, but attaching would nest a client ··· 2960 3011 }); 2961 3012 } 2962 3013 3014 + /** Strip `pty gc`'s flapping bookkeeping from a tag map before a manual 3015 + * restart / respawn. `strategy.status`, `strategy.consecutive-fast-fails`, 3016 + * `strategy.last-respawn-at`, and `strategy.command-hash` are all 3017 + * gc-owned — they exist to track auto-respawn state, and an operator 3018 + * action ("please try again") is a signal to reset them. Returns 3019 + * `undefined` when the input is missing/empty so downstream defaults 3020 + * (spawnDaemon skips the `tags` field entirely) apply. */ 3021 + function clearFlappingBookkeeping( 3022 + tags: Record<string, string> | undefined, 3023 + ): Record<string, string> | undefined { 3024 + if (!tags || Object.keys(tags).length === 0) return tags; 3025 + const out: Record<string, string> = {}; 3026 + for (const [k, v] of Object.entries(tags)) { 3027 + if (k === "strategy.status") continue; 3028 + if (k === "strategy.consecutive-fast-fails") continue; 3029 + if (k === "strategy.last-respawn-at") continue; 3030 + if (k === "strategy.command-hash") continue; 3031 + out[k] = v; 3032 + } 3033 + return Object.keys(out).length > 0 ? out : undefined; 3034 + } 3035 + 2963 3036 function strategyMarker(tags?: Record<string, string>): string { 2964 3037 if (!tags) return ""; 3038 + // Flapping supersedes permanent visually because it's what changed the 3039 + // operator's expectation ("gc stopped respawning this on purpose"). 3040 + // Rendered red so it stands out from the yellow [permanent]. 3041 + if (tags["strategy.status"] === "flapping") { 3042 + return " \x1b[31m[flapping]\x1b[0m"; 3043 + } 2965 3044 if (tags.strategy === "permanent") return " \x1b[33m[permanent]\x1b[0m"; 2966 3045 return ""; 2967 3046 }

+223

tests/gc-flap-clear-badge-root-len.test.ts

··· 1 + // Follow-ups to #56: 2 + // 1. `pty restart` clears strategy.status=flapping + bookkeeping. 3 + // 2. `pty up` clears the same on the "already running, tag-sync" path. 4 + // 3. `pty list` renders `[flapping]` badge (mutually exclusive with [permanent]). 5 + // 4. Fail-loud startup backstop when PTY_ROOT is too long for the socket-path 6 + // kernel limit — errors before any subcommand runs. 7 + 8 + import { describe, it, expect, afterEach, afterAll } from "vitest"; 9 + import * as fs from "node:fs"; 10 + import * as os from "node:os"; 11 + import * as path from "node:path"; 12 + import { fileURLToPath } from "node:url"; 13 + import { spawnSync } from "node:child_process"; 14 + 15 + const __dirname = path.dirname(fileURLToPath(import.meta.url)); 16 + const nodeBin = process.execPath; 17 + const cliPath = path.join(__dirname, "..", "dist", "cli.js"); 18 + 19 + const testRoot = fs.mkdtempSync(path.join(os.tmpdir(), "pty-follow-")); 20 + afterAll(() => { 21 + fs.rmSync(testRoot, { recursive: true, force: true, maxRetries: 3, retryDelay: 100 }); 22 + }); 23 + 24 + let sessionDirs: string[] = []; 25 + function makeSessionDir(): string { 26 + const dir = fs.mkdtempSync(path.join(testRoot, "d-")); 27 + sessionDirs.push(dir); 28 + return dir; 29 + } 30 + 31 + let nameCounter = 0; 32 + function uniqueName(): string { 33 + return `fc${++nameCounter}${Math.random().toString(36).slice(2, 5)}`; 34 + } 35 + 36 + function runCli(sessionDir: string, ...args: string[]) { 37 + return spawnSync(nodeBin, [cliPath, ...args], { 38 + env: { ...process.env, PTY_SESSION_DIR: sessionDir }, 39 + encoding: "utf-8", 40 + timeout: 15000, 41 + }); 42 + } 43 + 44 + function readMeta(sessionDir: string, name: string): any { 45 + return JSON.parse(fs.readFileSync(path.join(sessionDir, `${name}.json`), "utf-8")); 46 + } 47 + 48 + /** Write metadata simulating an exited session that gc previously marked 49 + * flapping. Includes all four bookkeeping tags. */ 50 + function writeFlappingExited( 51 + sessionDir: string, 52 + name: string, 53 + extraTags: Record<string, string> = {}, 54 + ): void { 55 + fs.writeFileSync(path.join(sessionDir, `${name}.json`), JSON.stringify({ 56 + command: "sh", args: ["-c", "exit 1"], displayCommand: "sh -c 'exit 1'", 57 + cwd: os.tmpdir(), 58 + createdAt: new Date(Date.now() - 10 * 60_000).toISOString(), 59 + exitedAt: new Date().toISOString(), 60 + exitCode: 1, 61 + tags: { 62 + strategy: "permanent", 63 + "strategy.status": "flapping", 64 + "strategy.consecutive-fast-fails": "3", 65 + "strategy.last-respawn-at": new Date(Date.now() - 5000).toISOString(), 66 + "strategy.command-hash": "0123456789abcdef", 67 + ...extraTags, 68 + }, 69 + })); 70 + const evPath = path.join(sessionDir, `${name}.events.jsonl`); 71 + if (!fs.existsSync(evPath)) fs.writeFileSync(evPath, ""); 72 + } 73 + 74 + afterEach(() => { 75 + for (const dir of sessionDirs) { 76 + try { 77 + for (const e of fs.readdirSync(dir)) { try { fs.unlinkSync(path.join(dir, e)); } catch {} } 78 + } catch {} 79 + } 80 + sessionDirs = []; 81 + }); 82 + 83 + describe("pty restart clears strategy.status=flapping + bookkeeping", () => { 84 + it("restart of a flapping-exited session drops all four gc bookkeeping tags", async () => { 85 + const dir = makeSessionDir(); 86 + const name = uniqueName(); 87 + writeFlappingExited(dir, name, { role: "test" }); 88 + 89 + // Restart it. -y skips the prompt; command is "sh -c 'exit 1'" which 90 + // will exit almost immediately — that's fine, we care about the tag 91 + // snapshot right after spawn. 92 + const r = runCli(dir, "restart", "-y", name); 93 + expect(r.status).toBe(0); 94 + 95 + // Give the daemon a moment to write its metadata. 96 + await new Promise((res) => setTimeout(res, 300)); 97 + 98 + const meta = readMeta(dir, name); 99 + expect(meta.tags.strategy).toBe("permanent"); 100 + expect(meta.tags.role).toBe("test"); 101 + // Bookkeeping tags are gone. 102 + expect(meta.tags["strategy.status"]).toBeUndefined(); 103 + expect(meta.tags["strategy.consecutive-fast-fails"]).toBeUndefined(); 104 + expect(meta.tags["strategy.last-respawn-at"]).toBeUndefined(); 105 + expect(meta.tags["strategy.command-hash"]).toBeUndefined(); 106 + }); 107 + }); 108 + 109 + describe("pty list renders [flapping] badge", () => { 110 + it("running session with strategy.status=flapping shows [flapping] in text output", () => { 111 + const dir = makeSessionDir(); 112 + const name = uniqueName(); 113 + // Simulate a running session (synthetic pid + no exitedAt) with a 114 + // flapping mark. `pty list` classifies status by (pid alive check + 115 + // exited fields); we cheat by using our own pid so isProcessAlive 116 + // returns true — the row renders as running. 117 + fs.writeFileSync(path.join(dir, `${name}.json`), JSON.stringify({ 118 + command: "sh", args: [], displayCommand: "sh", 119 + cwd: os.tmpdir(), 120 + createdAt: new Date().toISOString(), 121 + tags: { 122 + strategy: "permanent", 123 + "strategy.status": "flapping", 124 + }, 125 + })); 126 + fs.writeFileSync(path.join(dir, `${name}.pid`), String(process.pid)); 127 + 128 + const r = runCli(dir, "list"); 129 + expect(r.status).toBe(0); 130 + // ANSI color-code stripped for the assertion; check the literal marker. 131 + // `[flapping]` supersedes `[permanent]` when both would apply. 132 + expect(r.stdout).toContain("[flapping]"); 133 + expect(r.stdout).not.toContain("[permanent]"); 134 + }); 135 + 136 + it("session without strategy.status=flapping still shows [permanent]", () => { 137 + const dir = makeSessionDir(); 138 + const name = uniqueName(); 139 + fs.writeFileSync(path.join(dir, `${name}.json`), JSON.stringify({ 140 + command: "sh", args: [], displayCommand: "sh", 141 + cwd: os.tmpdir(), 142 + createdAt: new Date().toISOString(), 143 + tags: { strategy: "permanent" }, 144 + })); 145 + fs.writeFileSync(path.join(dir, `${name}.pid`), String(process.pid)); 146 + 147 + const r = runCli(dir, "list"); 148 + expect(r.status).toBe(0); 149 + expect(r.stdout).toContain("[permanent]"); 150 + expect(r.stdout).not.toContain("[flapping]"); 151 + }); 152 + }); 153 + 154 + describe("PTY_ROOT length backstop", () => { 155 + it("errors at startup when PTY_ROOT is too deep to fit the sockaddr_un limit", () => { 156 + // Build a 95-byte path — well past the 90-byte usable threshold 157 + // (104 − 14 for `/xxxxxxxx.sock`). Doesn't need to actually exist; 158 + // the check is on byte length, not existence. 159 + const tooLong = "/tmp/" + "a".repeat(95); 160 + expect(Buffer.byteLength(tooLong, "utf-8")).toBeGreaterThan(90); 161 + 162 + const r = spawnSync(nodeBin, [cliPath, "list"], { 163 + env: { ...process.env, PTY_ROOT: tooLong, PTY_ROOT_LEGACY_SILENT: "1" }, 164 + encoding: "utf-8", 165 + timeout: 5000, 166 + }); 167 + expect(r.status).not.toBe(0); 168 + expect(r.stderr).toMatch(/PTY_ROOT is too long/); 169 + expect(r.stderr).toMatch(/104-byte kernel limit/); 170 + // Points the finger at the root, not the name. 171 + expect(r.stderr).toMatch(/Shorten the root/); 172 + }); 173 + 174 + it("errors before any subcommand-specific parsing runs", () => { 175 + // Backstop should fire even on a bogus subcommand — the too-long 176 + // root is caught before dispatch. 177 + const tooLong = "/tmp/" + "b".repeat(100); 178 + const r = spawnSync(nodeBin, [cliPath, "definitely-not-a-real-subcommand"], { 179 + env: { ...process.env, PTY_ROOT: tooLong, PTY_ROOT_LEGACY_SILENT: "1" }, 180 + encoding: "utf-8", 181 + timeout: 5000, 182 + }); 183 + expect(r.status).not.toBe(0); 184 + expect(r.stderr).toMatch(/PTY_ROOT is too long/); 185 + // The "unknown command" path is NOT hit; the root check errors first. 186 + expect(r.stderr).not.toMatch(/Unknown command/); 187 + }); 188 + 189 + it("allows a root right at the usable threshold", () => { 190 + // Build a root at exactly 90 bytes — the maximum that leaves room 191 + // for `/xxxxxxxx.sock` in 104. This should succeed (empty list). 192 + const usable = 104 - ("/".length + 8 + ".sock".length); 193 + const okRoot = "/tmp/" + "c".repeat(usable - "/tmp/".length); 194 + expect(Buffer.byteLength(okRoot, "utf-8")).toBe(usable); 195 + fs.mkdirSync(okRoot, { recursive: true }); 196 + try { 197 + const r = spawnSync(nodeBin, [cliPath, "list", "--json"], { 198 + env: { ...process.env, PTY_ROOT: okRoot, PTY_ROOT_LEGACY_SILENT: "1" }, 199 + encoding: "utf-8", 200 + timeout: 5000, 201 + }); 202 + expect(r.status).toBe(0); 203 + expect(JSON.parse(r.stdout)).toEqual([]); 204 + } finally { 205 + try { fs.rmSync(okRoot, { recursive: true, force: true }); } catch {} 206 + } 207 + }); 208 + 209 + it("--root <shorter> overrides an env that would otherwise fail", () => { 210 + // Env is too long; --root override is fine. The startup check reads 211 + // process.env.PTY_ROOT *after* --root parsing has set it, so the 212 + // override wins. 213 + const tooLongEnv = "/tmp/" + "d".repeat(95); 214 + const shortFlag = fs.mkdtempSync(path.join(testRoot, "shorter-")); 215 + const r = spawnSync(nodeBin, [cliPath, "--root", shortFlag, "list", "--json"], { 216 + env: { ...process.env, PTY_ROOT: tooLongEnv, PTY_ROOT_LEGACY_SILENT: "1" }, 217 + encoding: "utf-8", 218 + timeout: 5000, 219 + }); 220 + expect(r.status).toBe(0); 221 + expect(JSON.parse(r.stdout)).toEqual([]); 222 + }); 223 + });

Configure Feed

Configure Feed