fix(gc): guard cleanup by observed generation

agent-session-id: dev3.dotfiles-cos-misc-agent-runtime-simplification
agent-tool: Codex
agent-tool-version: 0.145.0
agent-model: gpt-5.6-sol
agent-runtime-profile: /home/schickling/.config/coding-agents/profile.json
agent-skills-manifest: /nix/store/nk9iml2841l1yjjg0f6f0d3y60zkg1nn-agent-skills-corpus/share/agent-skills/manifest.json
tooling-profile: dotfiles@4a0515f

Author: schickling-assistant

CHANGELOG.md

@@ -16,6 +16,15 @@
   when metadata is missing or malformed, and revalidates it while holding the
   per-name creation lock before removal.
 
+### Generation-guarded gc cleanup and respawn
+
+- Residual sweep and permanent respawn now compare the generation observed by
+  gc with the current metadata while holding the per-name creation lock.
+  Cleanup is skipped when another process replaced or edited the record after
+  gc's snapshot, preventing a stale pass from unlinking or respawning over the
+  replacement. Legacy generation-less records use exact metadata equality as a
+  conservative fallback.
+
 ### Attach-only CLI policy
 
 - `pty attach --no-restart <ref>` attaches only to a currently running

src/sessions.ts

@@ -449,6 +449,50 @@ export async function cleanupRawCandidateGuarded(
   }
 }
 
+function metadataMatchesObservation(
+  observed: SessionMetadata,
+  current: SessionMetadata,
+): boolean {
+  if (observed.generation !== undefined || current.generation !== undefined) {
+    return observed.generation !== undefined &&
+      observed.generation === current.generation;
+  }
+  // Legacy records have no generation token. Exact structural equality is a
+  // conservative fallback: any intervening tag/launch/exit update makes the
+  // observation stale and suppresses cleanup.
+  return JSON.stringify(observed) === JSON.stringify(current);
+}
+
+function cleanupAllWhileLocked(name: string): void {
+  cleanupSocket(name);
+  try {
+    fs.unlinkSync(getMetadataPath(name));
+  } catch {}
+  try {
+    fs.unlinkSync(getEventsPath(name));
+  } catch {}
+}
+
+/** @internal Generation-CAS cleanup primitive; not part of client-api. */
+export async function cleanupObservedSession(
+  session: SessionInfo,
+): Promise<boolean> {
+  if (!session.metadata || !acquireLock(session.name)) return false;
+  try {
+    const current = readMetadata(session.name);
+    if (
+      !current ||
+      !metadataMatchesObservation(session.metadata, current)
+    ) {
+      return false;
+    }
+    cleanupAllWhileLocked(session.name);
+    return true;
+  } finally {
+    releaseLock(session.name);
+  }
+}
+
 /** Return one bounded, read-only observation of the session registry.
  *
  * This function deliberately performs no lifecycle work: it does not create
@@ -980,8 +1024,9 @@ export async function gc(
     }
 
     try {
-      await respawnPermanent(s.name, s.metadata!, decision.newBookkeeping);
-      respawned.push({ name: s.name, ptyfileReread });
+      if (await respawnPermanent(s.name, s.metadata!, decision.newBookkeeping)) {
+        respawned.push({ name: s.name, ptyfileReread });
+      }
     } catch (err: any) {
       respawnFailed.push({ name: s.name, error: err?.message ?? String(err) });
     }
@@ -1006,8 +1051,11 @@ export async function gc(
       kept.push(s.name);
       continue;
     }
-    if (!dryRun) cleanupAll(s.name);
-    removed.push(s.name);
+    if (dryRun) {
+      removed.push(s.name);
+    } else if (await cleanupObservedSession(s)) {
+      removed.push(s.name);
+    }
   }
 
   return {
@@ -1207,12 +1255,14 @@ function classifyFlapping(
  *
  *  Lazy-imports `spawn.ts` so the `sessions.ts ↔ spawn.ts` cycle doesn't
  *  bite at module-init time. After spawn, appends a `session_respawn`
- *  event to the session's event log so consumers see the restart. */
-async function respawnPermanent(
+ *  event to the session's event log so consumers see the restart.
+ *
+ *  @internal Generation-CAS primitive; not part of client-api. */
+export async function respawnPermanent(
   name: string,
   metadata: SessionMetadata,
   bookkeepingOverlay: Record<string, string> = {},
-): Promise<void> {
+): Promise<boolean> {
   let command = metadata.command;
   let args = metadata.args;
   let displayCommand = metadata.displayCommand;
@@ -1261,22 +1311,35 @@ async function respawnPermanent(
     delete tags["strategy.status"];
   }
 
-  // Wipe stale socket/pid/events before respawn so spawnDaemon doesn't
-  // trip over leftovers from the dead daemon. Metadata is recreated by
-  // spawnDaemon.
-  cleanupAll(name);
-
-  const { spawnDaemon } = await import("./spawn.ts");
-  await spawnDaemon({
-    name, command, args, displayCommand, cwd, tags,
-    ...(displayName ? { displayName } : {}),
-    ...(metadata.rows !== undefined ? { rows: metadata.rows } : {}),
-    ...(metadata.cols !== undefined ? { cols: metadata.cols } : {}),
-    ...(metadata.ephemeral !== undefined ? { ephemeral: metadata.ephemeral } : {}),
-    ...(metadata.isolateEnv ? { isolateEnv: true } : {}),
-    ...(extraEnv && Object.keys(extraEnv).length > 0 ? { extraEnv } : {}),
-    ...(exactEnv ? { env: exactEnv } : {}),
-  });
+  // Serialize compare-and-swap cleanup + replacement creation under the same
+  // per-name lock. If the observed generation changed while gc was planning,
+  // this tick is stale and must not touch the replacement.
+  if (!acquireLock(name)) return false;
+  try {
+    const current = readMetadata(name);
+    if (!current || !metadataMatchesObservation(metadata, current)) {
+      return false;
+    }
+
+    // Wipe stale socket/pid/events before respawn so spawnDaemon doesn't trip
+    // over leftovers from the dead daemon. Keep the creation lock held until
+    // the replacement has published its socket.
+    cleanupAllWhileLocked(name);
+
+    const { spawnDaemon } = await import("./spawn.ts");
+    await spawnDaemon({
+      name, command, args, displayCommand, cwd, tags,
+      ...(displayName ? { displayName } : {}),
+      ...(metadata.rows !== undefined ? { rows: metadata.rows } : {}),
+      ...(metadata.cols !== undefined ? { cols: metadata.cols } : {}),
+      ...(metadata.ephemeral !== undefined ? { ephemeral: metadata.ephemeral } : {}),
+      ...(metadata.isolateEnv ? { isolateEnv: true } : {}),
+      ...(extraEnv && Object.keys(extraEnv).length > 0 ? { extraEnv } : {}),
+      ...(exactEnv ? { env: exactEnv } : {}),
+    });
+  } finally {
+    releaseLock(name);
+  }
 
   // Best-effort event; respawn already succeeded if we got here.
   try {
@@ -1286,6 +1349,7 @@ async function respawnPermanent(
       ts: new Date().toISOString(),
     });
   } catch {}
+  return true;
 }
 
 /**

tests/gc-generation-guard.test.ts

@@ -0,0 +1,87 @@
+import * as fs from "node:fs";
+import * as os from "node:os";
+import * as path from "node:path";
+import { afterEach, describe, expect, it } from "vitest";
+import {
+  cleanupObservedSession,
+  respawnPermanent,
+  type SessionInfo,
+  type SessionMetadata,
+} from "../src/sessions.ts";
+
+const roots: string[] = [];
+
+const makeRoot = () => {
+  const root = fs.mkdtempSync(path.join(os.tmpdir(), "pty-gc-generation-"));
+  roots.push(root);
+  process.env.PTY_ROOT = root;
+  return root;
+};
+
+const metadata = (
+  root: string,
+  generation: string,
+  tags?: Record<string, string>,
+): SessionMetadata => ({
+  generation,
+  daemonPid: 2147483647,
+  command: "true",
+  args: [],
+  displayCommand: "true",
+  cwd: root,
+  createdAt: "2026-01-01T00:00:00.000Z",
+  exitedAt: "2026-01-01T00:00:01.000Z",
+  exitCode: 0,
+  tags,
+});
+
+afterEach(() => {
+  delete process.env.PTY_ROOT;
+  for (const root of roots.splice(0)) {
+    fs.rmSync(root, { recursive: true, force: true });
+  }
+});
+
+describe("gc generation compare-and-swap", () => {
+  it("does not residual-sweep a replacement generation", async () => {
+    const root = makeRoot();
+    const name = "residual";
+    const observed = metadata(root, "old");
+    fs.writeFileSync(
+      path.join(root, `${name}.json`),
+      JSON.stringify(metadata(root, "replacement")),
+    );
+    const session: SessionInfo = {
+      name,
+      socketPath: path.join(root, `${name}.sock`),
+      pid: null,
+      status: "exited",
+      metadata: observed,
+    };
+
+    const removed = await cleanupObservedSession(session);
+
+    expect(removed).toBe(false);
+    expect(JSON.parse(
+      fs.readFileSync(path.join(root, `${name}.json`), "utf-8"),
+    ).generation).toBe("replacement");
+  });
+
+  it("does not respawn over a replacement generation", async () => {
+    const root = makeRoot();
+    const name = "permanent";
+    const tags = { strategy: "permanent" };
+    const observed = metadata(root, "old", tags);
+    fs.writeFileSync(
+      path.join(root, `${name}.json`),
+      JSON.stringify(metadata(root, "replacement", tags)),
+    );
+
+    const respawned = await respawnPermanent(name, observed);
+
+    expect(respawned).toBe(false);
+    expect(JSON.parse(
+      fs.readFileSync(path.join(root, `${name}.json`), "utf-8"),
+    ).generation).toBe("replacement");
+  });
+});