fix(C4): record a crashed session's live child pid, never signal it

Council item C4 (SIGNOFF-M2/ARBITRATION.md, residual/recorded). A server
restart with the PTY/ACP child still alive was neither tested nor
handled: reclaim logged and started a second agent, leaving the orphan
running with no trace of it anywhere.

- PTYTransport.pid / ACPTransport.pid: the forked child / subprocess pid,
  None before start(). Purely accessor properties -- nothing reads or
  acts on them beyond what follows.
- build_session_state_payload gains an optional child_pid param, recorded
  on the claim (informational only).
- Both web start paths capture acquire()'s return value and pass
  getattr(active_session.transport, "pid", None) through (None for
  StubTransport in tests, matching every other test's existing shape).
- session_state.reclaim_log_message(state): the single place both start
  paths now build the "reclaiming a stale claim" line (replacing the
  duplicated inline format string R-01b required stay in exact sync),
  extended to name child_pid when the reclaimed claim had one.

Rejected, per the council's own reasoning: killing the child on reclaim.
Pid reuse could mean it now points at an unrelated process, and the
child may be the user's own still-useful agent that simply outlived a
crashed web server -- neither risk is worth taking. This is visibility
only, feeding a future clean/doctor orphan report (R-01g, 0.2.0).

Tests: PTYTransport/ACPTransport .pid unit tests, reclaim_log_message
unit tests, and an end-to-end reclaim test with a tracking os.kill wrapper
proving no real signal (only sig=0 liveness probes) ever reaches the
recorded child pid, while the reclaim log names it.

Evidence: reviews/2026-09-02-full-repo-review/evidence/M2/step-9/C4/.

Co-Authored-By: Claude Fable 5.1 <noreply@anthropic.com>

Author: NetDevAutomate

CHANGELOG.md

@@ -186,6 +186,10 @@ experience may change before `1.0.0`.
   moment the poll runs; it now reads first and treats a vanished file as
   "no update" instead of checking existence then reading as two separate
   steps (C7).
+- A reclaimed session's log line now names the crashed session's agent
+  process (if one was recorded and is still running) so it is visible
+  that an orphaned agent may still be alive on the machine; StudyLoop
+  still never signals it (C4).
 - Ending a session -- from the Web UI, `studyloop study --end`, or the study
   sidebar's End Session key -- no longer terminates every other study
   session on the machine. Each end path now closes only its own terminal

docs/architecture/session-authority.md

@@ -44,6 +44,22 @@ session's leftover topics/parking stayed visible to whoever reclaimed the
 slot. A live (blocking) claim's files are never touched -- clearing is a
 reclaim-only side effect.
 
+**C4 (council, residual, recorded not fixed):** a reclaim never checks --
+and never kills -- a live PTY child the crashed server's process left
+running (a "server restart with the agent still alive" case). Both
+`PTYTransport` and `ACPTransport` now expose their child's pid via a
+`.pid` property; the web start paths read it via `getattr(transport,
+"pid", None)` (`None` for `StubTransport`, used in tests) and record it on
+the claim as `child_pid` (`build_session_state_payload`). A reclaim's log
+line (`session_state.reclaim_log_message`) names it when the PREVIOUS
+claim had one, so a human (or a future `clean`/`doctor` orphan report,
+item R-01g, 0.2.0) has it to hand -- reclaim itself takes no action on it.
+Rejected: killing it on reclaim. The pid could have been reused by an
+unrelated process by the time this runs, and the child may be the user's
+own still-useful agent that simply outlived a crashed web server; neither
+risk is worth taking to tidy up an orphan that costs nothing but a stray
+process entry.
+
 ## 3. Start matrix
 
 | | new start: CLI | new start: web (pty/acp) |

packages/studyloop/src/studyloop/session/start.py

@@ -276,6 +276,7 @@ def start_session(
         claim_blocks_cli_start,
         clear_session_files,
         read_session_state,
+        reclaim_log_message,
         write_session_state,
     )
 
@@ -329,11 +330,7 @@ def start_session(
         # C2: same rule as the web path -- a reclaimed session must not
         # inherit the dead session's topics/parking.
         clear_session_files()
-        logger.warning(
-            "Reclaiming stale session claim id=%s transport=%s — its owner is no longer alive",
-            claim.get("study_session_id"),
-            claim.get("transport", "cli"),
-        )
+        logger.warning(reclaim_log_message(claim))
 
     # --- Create DB session ---
 

packages/studyloop/src/studyloop/session/transports/acp.py

@@ -165,6 +165,14 @@ def __init__(
         self._stderr_tail: bytes = b""
         self._stderr_task: asyncio.Task[None] | None = None
 
+    @property
+    def pid(self) -> int | None:
+        """The subprocess's pid once ``start()`` has run, else ``None``.
+
+        Same accessor and purpose as ``PTYTransport.pid`` -- see there.
+        """
+        return self._state.proc.pid if self._state is not None else None
+
     # ---- AgentSessionTransport ------------------------------------------
 
     async def start(self, config: SessionConfig) -> None:

packages/studyloop/src/studyloop/session/transports/pty.py

@@ -225,6 +225,17 @@ def __init__(
         self._ended = False
         self._cancel_requested = False
 
+    @property
+    def pid(self) -> int | None:
+        """The forked child's pid once ``start()`` has run, else ``None``.
+
+        C4 (council): the route layer reads this after ``acquire()``
+        succeeds to record it on the claim as ``child_pid`` -- purely
+        informational (for `clean`/`doctor`'s future orphan reporting,
+        R-01g); nothing in this lane signals or kills it.
+        """
+        return self._state.pid if self._state is not None else None
+
     # ---- AgentSessionTransport ---------------------------------------------
 
     async def start(self, config: SessionConfig) -> None:

packages/studyloop/src/studyloop/session_state.py

@@ -259,6 +259,27 @@ def is_session_active() -> bool:
     return state.get("mode") != "ended"
 
 
+def reclaim_log_message(state: dict) -> str:
+    """The "reclaiming a stale claim" warning both start paths log,
+    identically (R-01b required exact-wording parity between the CLI and
+    web paths; building it once here, rather than duplicating it, keeps
+    that guarantee mechanical instead of a promise to remember).
+
+    C4 (council): appends the previous owner's recorded ``child_pid`` when
+    present, so a human (or `clean`/`doctor`, R-01g, 0.2.0) reading the log
+    has it to hand -- reclaim itself never acts on ``child_pid`` (pid
+    reuse makes killing an old child unsafe, and it may be the user's own
+    still-useful agent).
+    """
+    session_id = state.get("study_session_id")
+    transport = state.get("transport", "cli")
+    child_pid = state.get("child_pid")
+    base = f"Reclaiming stale session claim id={session_id} transport={transport}"
+    if child_pid is not None:
+        base += f" child_pid={child_pid}"
+    return base + " — its owner is no longer alive"
+
+
 def _claim_exists(state: dict) -> bool:
     """Whether ``state`` names a claim that hasn't been explicitly ended."""
     return bool(state.get("study_session_id")) and state.get("mode") != "ended"

packages/studyloop/src/studyloop/web/routes/session/_start.py

@@ -143,11 +143,7 @@ async def _session_conflict() -> JSONResponse | None:
             # here: nothing else can claim the slot until this function
             # returns None and the caller's own write lands).
             session_state.clear_session_files()
-            logger.warning(
-                "Reclaiming stale session claim id=%s transport=%s — its owner is no longer alive",
-                state.get("study_session_id"),
-                state.get("transport", "cli"),
-            )
+            logger.warning(session_state.reclaim_log_message(state))
         return None
 
     session_id = str(state.get("study_session_id"))
@@ -349,7 +345,7 @@ async def _start_pty_session(
     factory = session_pkg._build_pty_transport(config)
 
     try:
-        await session_active.acquire(config, factory)
+        active_session = await session_active.acquire(config, factory)
     except SessionAlreadyActiveError:
         from studyloop.history import abort_study_session
 
@@ -391,6 +387,12 @@ async def _start_pty_session(
             persona_hash=persona_hash,
             transport="pty",
             now=datetime.now(UTC),
+            # C4 (council): informational only -- getattr because
+            # StubTransport (tests) exposes no .pid, and the real
+            # PTYTransport's own .pid property returns None before
+            # start() runs (never the case here, after a successful
+            # acquire, but the getattr default is the same either way).
+            child_pid=getattr(active_session.transport, "pid", None),
         )
         # origin distinguishes Study Session ('study') from Body Double
         # ('body-double') starts. Merged in here rather than in
@@ -574,7 +576,7 @@ async def _start_acp_session(
     factory = session_pkg._build_acp_transport(config)
 
     try:
-        await session_active.acquire(config, factory)
+        active_session = await session_active.acquire(config, factory)
     except SessionAlreadyActiveError:
         from studyloop.history import abort_study_session
 
@@ -615,6 +617,8 @@ async def _start_acp_session(
             persona_hash=persona_hash,
             transport="acp",
             now=datetime.now(UTC),
+            # C4 (council): see the PTY path's identical comment.
+            child_pid=getattr(active_session.transport, "pid", None),
         )
         # See PTY path: origin merged here, not in build_session_state_payload.
         acp_state["origin"] = origin

packages/studyloop/src/studyloop/web/services/session_start.py

@@ -51,6 +51,7 @@ def build_session_state_payload(
     transport: TransportName,
     now: datetime,
     persona_file: str | None = None,
+    child_pid: int | None = None,
 ) -> dict[str, object]:
     """Build the common state payload for PTY and ACP web session starts."""
     timestamp = now.isoformat()
@@ -88,4 +89,10 @@ def build_session_state_payload(
     }
     if persona_file is not None:
         payload["persona_file"] = persona_file
+    if child_pid is not None:
+        # C4 (council): the transport's own child process pid, purely
+        # informational -- for `clean`/`doctor`'s future orphan reporting
+        # (R-01g, 0.2.0). A reclaim never signals it (see
+        # session_state.reclaim_log_message's docstring for why).
+        payload["child_pid"] = child_pid
     return payload

packages/studyloop/tests/test_acp_transport.py

@@ -86,6 +86,31 @@ async def _reset_env(monkeypatch):
 
 
 class TestStart:
+    @pytest.mark.asyncio
+    async def test_pid_property_reflects_the_subprocess(self, tmp_path, monkeypatch, _reset_env):
+        """C4: same accessor as PTYTransport, for the same reason (child_pid
+        recorded on the claim, for future orphan reporting -- R-01g)."""
+        monkeypatch.setenv(
+            "STUB_ACP_INIT_RESULT",
+            json.dumps(
+                {
+                    "protocolVersion": 1,
+                    "agentCapabilities": {"loadSession": True},
+                    "authMethods": [],
+                    "agentInfo": {"name": "Stub ACP Agent", "version": "0.0.1"},
+                }
+            ),
+        )
+        transport = ACPTransport(
+            resolve_binary=_stub_resolve_binary,
+            build_argv=_stub_build_argv(),
+        )
+        assert transport.pid is None
+        await transport.start(_make_config(tmp_path))
+        assert isinstance(transport.pid, int)
+        assert transport.pid > 0
+        await transport.end()
+
     @pytest.mark.asyncio
     async def test_start_sends_initialize_and_session_new(self, tmp_path, monkeypatch, _reset_env):
         """Happy path: initialize + session/new + Started event with the

packages/studyloop/tests/test_pty_transport.py

@@ -211,6 +211,17 @@ async def _drain() -> None:
 
 @pytest.mark.asyncio
 class TestHappyPath:
+    async def test_pid_property_reflects_the_forked_child(
+        self, transport: PTYTransport, config: SessionConfig
+    ) -> None:
+        """C4: the route layer needs the child's pid, after start(), to
+        record it on the claim as child_pid -- for clean/doctor's future
+        orphan reporting (R-01g), never to kill it on reclaim."""
+        assert transport.pid is None
+        await transport.start(config)
+        assert isinstance(transport.pid, int)
+        assert transport.pid > 0
+
     async def test_start_emits_started_event_first(
         self, transport: PTYTransport, config: SessionConfig
     ) -> None: