fix(runtime): bind PTY teardown to spawn identity

Capture the PTY leader's ProcessIdentity and its own session group
immediately after spawn, while the child is held unreaped, and drive
teardown from that frozen record instead of re-deriving the group from
proc.pid. Every signal re-verifies the leader: OWNED and still leading
the group signals the group; GONE signals only the recorded group, never
the pid; FOREIGN proves the group's lifetime ended and nothing is
signalled; UNVERIFIABLE or a missing spawn identity signals nothing.
The server's own process group is never recorded or signalled.

routes/shell_routes.py

@@ -695,7 +695,7 @@ def _session_pgid(pid: int) -> int | None:
     return pgid
 
 
-def _signal_session(pgid: int | None, pid: int, sig: int) -> bool:
+def _signal_session(pgid: int | None, pid: int | None, sig: int) -> bool:
     """Send ``sig`` to the whole process group, or to the lone process.
 
     Returns whether anything was signalled, so a caller can tell "the session
@@ -721,6 +721,27 @@ def _session_alive(pgid: int | None, pid: int) -> bool:
     return pid_alive(pid)
 
 
+def _bind_pty_spawn_identity(proc) -> None:
+    """Freeze the PTY leader's identity and its session group at spawn.
+
+    Called immediately after the spawn, while the pid is known to be the child
+    just created: we hold it unreaped, so the slot cannot have been reissued.
+    The group is recorded only when it is the leader's own (``setsid``
+    applied: pgid == pid) and the identity still verifies after reading it.
+    Teardown works from this record alone and never re-derives ownership
+    from ``proc.pid``, which outlives the process it named.
+    """
+    pid = getattr(proc, "pid", None)
+    if not pid:
+        return
+    identity = process_lifecycle.ProcessIdentity.capture(pid)
+    pgid = _session_pgid(pid)
+    if pgid != pid or identity.verdict() != process_lifecycle.OWNED:
+        pgid = None  # No safe session group: teardown reaches the child alone.
+    proc._ody_pty_identity = process_lifecycle.ProcessIdentity(
+        pid=identity.pid, start_token=identity.start_token, pgid=pgid)
+
+
 async def _terminate_pty_session(proc) -> bool:
     """Kill the PTY child and every process in the session it leads.
 
@@ -732,6 +753,18 @@ async def _terminate_pty_session(proc) -> bool:
     outlives the grace period, and return whether the session is actually gone
     so the caller can say so rather than assume it.
 
+    Ownership is the identity frozen at spawn (:func:`_bind_pty_spawn_identity`),
+    re-verified before every signal:
+
+    * leader OWNED and still leading the recorded group → signal the group;
+    * leader GONE (exited and reaped) → the recorded group only, never the
+      pid: a group id is not reissued while the group lives, so a present
+      group with no process in its leader's slot is still ours;
+    * leader FOREIGN → the pid was reissued, which proves our group's
+      lifetime had already ended; nothing of ours is left to signal;
+    * leader UNVERIFIABLE, or no spawn identity at all → nothing is
+      signalled and the session is not reported gone.
+
     The ladder itself is :func:`src.process_lifecycle.escalate_async`; the
     leader is reaped through ``proc.wait()`` inside each window, otherwise its
     own zombie keeps the group alive and the probe can never come back clean.
@@ -739,15 +772,41 @@ async def _terminate_pty_session(proc) -> bool:
     pid = getattr(proc, "pid", None)
     if pid is None:
         return True
-    pgid = _session_pgid(pid)
+    frozen = getattr(proc, "_ody_pty_identity", None)
+    if frozen is None:
+        logger.warning("PTY teardown for pid %s has no spawn identity; not signalling", pid)
+        return False
+    pgid = frozen.pgid
+
+    def _gone() -> bool:
+        verdict = frozen.verdict()
+        if verdict == process_lifecycle.FOREIGN:
+            return True
+        if verdict == process_lifecycle.UNVERIFIABLE:
+            return False
+        if pgid is not None:
+            return not _session_alive(pgid, frozen.pid)
+        return verdict == process_lifecycle.GONE or process_lifecycle.is_zombie(frozen.pid)
+
+    def _send(sig) -> bool:
+        verdict = frozen.verdict()
+        if verdict == process_lifecycle.OWNED:
+            if pgid is not None and process_lifecycle.pgid_of(frozen.pid) == pgid:
+                return _signal_session(pgid, frozen.pid, sig)
+            # Child-only: no safe group, or the leader no longer leads it.
+            return _signal_session(None, frozen.pid, sig)
+        if verdict == process_lifecycle.GONE and pgid is not None:
+            # Never fall back to the pid: it names no process of ours now.
+            return _signal_session(pgid, None, sig)
+        return False
 
     async def _reap_leader():
         if proc.returncode is None:
             await proc.wait()
 
     result = await process_lifecycle.escalate_async(
-        lambda: not _session_alive(pgid, pid),
-        lambda sig: _signal_session(pgid, pid, sig),
+        _gone,
+        _send,
         steps=tuple((sig, PTY_KILL_GRACE) for sig in PTY_KILL_ESCALATION),
         wait=_reap_leader,
         poll_s=PTY_KILL_POLL_INTERVAL,
@@ -801,6 +860,7 @@ async def _generate_pty(cmd: str, timeout: int, request: Request):
         cwd=str(Path.home()),
         preexec_fn=os.setsid,
     )
+    _bind_pty_spawn_identity(proc)
     os.close(slave_fd)  # parent doesn't need the slave side
 
     deadline = (loop.time() + timeout) if timeout else None

tests/test_shell_routes.py

@@ -119,13 +119,31 @@ pty_session = pytest.mark.skipif(
 
 
 async def _spawn_pty_style_session(script: str):
-    """Spawn `script` the way _generate_pty does: its own session via setsid."""
-    return await asyncio.create_subprocess_shell(
+    """Spawn `script` the way _generate_pty does: its own session via setsid,
+    with the leader's identity and group bound at spawn."""
+    import routes.shell_routes as shell_routes
+
+    proc = await asyncio.create_subprocess_shell(
         script,
         stdout=asyncio.subprocess.DEVNULL,
         stderr=asyncio.subprocess.DEVNULL,
         preexec_fn=os.setsid,
     )
+    shell_routes._bind_pty_spawn_identity(proc)
+    return proc
+
+
+def _bound_fake_leader(monkeypatch, pid=4242):
+    """A fake PTY leader whose spawn identity verifies and still leads its group."""
+    from src import process_lifecycle, process_ownership
+
+    real_getpgid = os.getpgid
+    monkeypatch.setattr(process_ownership, "verify", lambda p, token: process_ownership.OWNED)
+    monkeypatch.setattr(os, "getpgid", lambda p: pid if p == pid else real_getpgid(p))
+    return SimpleNamespace(
+        pid=pid, returncode=0, wait=None,
+        _ody_pty_identity=process_lifecycle.ProcessIdentity(pid, "spawn-token", pgid=pid),
+    )
 
 
 def _stubborn_child(pid_file: Path, ignore: tuple[str, ...]) -> str:
@@ -281,9 +299,8 @@ async def test_terminate_pty_session_reports_a_session_it_could_not_kill(
     monkeypatch.setattr(shell_routes, "PTY_KILL_GRACE", 0.01)
     monkeypatch.setattr(shell_routes, "_signal_session", lambda *_: True)
     monkeypatch.setattr(shell_routes, "_session_alive", lambda *_: True)
-    monkeypatch.setattr(shell_routes, "_session_pgid", lambda _: 4242)
 
-    proc = SimpleNamespace(pid=4242, returncode=0, wait=None)
+    proc = _bound_fake_leader(monkeypatch)
     assert await shell_routes._terminate_pty_session(proc) is False
 
 
@@ -293,7 +310,6 @@ async def test_terminate_pty_session_escalates_before_giving_up(monkeypatch):
 
     sent = []
     monkeypatch.setattr(shell_routes, "PTY_KILL_GRACE", 0.01)
-    monkeypatch.setattr(shell_routes, "_session_pgid", lambda _: 4242)
     monkeypatch.setattr(shell_routes, "_session_alive", lambda *_: True)
     monkeypatch.setattr(
         shell_routes,
@@ -301,7 +317,7 @@ async def test_terminate_pty_session_escalates_before_giving_up(monkeypatch):
         lambda pgid, pid, sig: sent.append(sig) or True,
     )
 
-    proc = SimpleNamespace(pid=4242, returncode=0, wait=None)
+    proc = _bound_fake_leader(monkeypatch)
     await shell_routes._terminate_pty_session(proc)
 
     assert sent == [signal.SIGTERM, signal.SIGKILL]
@@ -343,20 +359,60 @@ async def test_terminate_pty_session_never_signals_the_servers_own_group(monkeyp
     """If setsid did not apply, the child's group is ours: reach the child alone."""
     import routes.shell_routes as shell_routes
 
+    from src import process_ownership
+
     own = os.getpgid(0)
     sent = []
     monkeypatch.setattr(shell_routes, "PTY_KILL_GRACE", 0.01)
     monkeypatch.setattr(shell_routes.process_lifecycle, "pgid_of", lambda _pid: own)
-    monkeypatch.setattr(os, "killpg", lambda pgid, sig: sent.append(("group", pgid, sig)))
-    monkeypatch.setattr(os, "kill", lambda pid, sig: sent.append(("pid", pid, sig)))
+    monkeypatch.setattr(process_ownership, "start_token", lambda pid: "the-leader")
 
     proc = SimpleNamespace(pid=987654, returncode=0, wait=None)
     assert shell_routes._session_pgid(proc.pid) is None
+    shell_routes._bind_pty_spawn_identity(proc)
+    assert proc._ody_pty_identity.pgid is None  # no safe session group recorded
+
+    monkeypatch.setattr(os, "killpg", lambda pgid, sig: sent.append(("group", pgid, sig)))
+    monkeypatch.setattr(os, "kill", lambda pid, sig: sent.append(("pid", pid, sig)))
     await shell_routes._terminate_pty_session(proc)
 
     assert sent and all(kind == "pid" and target == 987654 for kind, target, _ in sent), sent
 
 
+@pytest.mark.skipif(os.name == "nt", reason="POSIX process groups")
+async def test_terminate_pty_session_never_signals_a_reused_leader_pid(monkeypatch):
+    """Leader spawned and bound → reaped → pid reissued → teardown signals nothing.
+
+    The replacement is the worst case: an unrelated session leader, so both
+    its pid and its process group carry the number our leader had.
+    """
+    import routes.shell_routes as shell_routes
+    from src import process_ownership
+
+    pid = 987650
+    real_getpgid = os.getpgid
+    occupant = {"token": "leader-token"}
+    monkeypatch.setattr(process_ownership, "start_token",
+                        lambda p: occupant["token"] if int(p) == pid else None)
+    monkeypatch.setattr(os, "getpgid", lambda p: pid if p == pid else real_getpgid(p))
+
+    proc = SimpleNamespace(pid=pid, returncode=None, wait=None)
+    shell_routes._bind_pty_spawn_identity(proc)  # spawn time: the leader we just created
+    assert proc._ody_pty_identity.pgid == pid
+
+    # The leader exits and is reaped; the kernel reissues its pid to a stranger.
+    proc.returncode = 0
+    occupant["token"] = "replacement-token"
+    signalled = []
+    monkeypatch.setattr(os, "killpg", lambda g, sig: sig and signalled.append(("group", g, sig)))
+    monkeypatch.setattr(os, "kill", lambda p, sig: sig and signalled.append(("pid", p, sig)))
+    monkeypatch.setattr(shell_routes, "PTY_KILL_GRACE", 0.01)
+
+    await shell_routes._terminate_pty_session(proc)
+
+    assert signalled == [], f"teardown signalled the replacement: {signalled}"
+
+
 def test_session_alive_treats_a_refused_probe_as_alive(monkeypatch):
     """EPERM says the group exists but we may not signal it, not that it died.
 
@@ -391,11 +447,10 @@ async def test_terminate_pty_session_reports_a_group_it_may_not_signal(monkeypat
         raise PermissionError(errno.EPERM, "Operation not permitted")
 
     monkeypatch.setattr(shell_routes, "PTY_KILL_GRACE", 0.01)
-    monkeypatch.setattr(shell_routes, "_session_pgid", lambda _: 4242)
+    proc = _bound_fake_leader(monkeypatch)
     monkeypatch.setattr(shell_routes.os, "killpg", refuse)
     monkeypatch.setattr(shell_routes.os, "kill", refuse)
 
-    proc = SimpleNamespace(pid=4242, returncode=0, wait=None)
     assert await shell_routes._terminate_pty_session(proc) is False